Hey all, I'm looking for advice on how to properly...
# how-do-i
t
Hey all, I'm looking for advice on how to properly adopt stacking. I've been introducing this to our organisation and people creating stacks are loving it, but people reviewing the stacks are hating it. There is no way to have a unified view of what changes a stack contains other than using compare to trunk and last PR of stack in Github, which doesn't allow for commenting. Most of the ones that use stacking have adopted a similar approach to atomic commits, where each PR in a stack is a self-contained change, which tends to create quite large stacks. Now I'm wondering if we're doing stacking wrong since reviews are supposed be easier and faster with stacks. Any tip or advice would be really appreciated
a
Hey! I'm surprised that people reviewing the stacks are hating it. It sounds like the main feature you guys are missing is the ability to look at an entire stack at once?
t
Possibly. I'm honestly not sure if that's the real reason, but it's the reason that gets stated
s
I'm fairly new to stacking so others might be able to offer better advice, but maybe it's that you're separating things excessively into separate PRs? One mental model that helps me make sense of stacks is that the Graphite UI lets you merge stacks partially, from the bottom. This means that once the say bottom 2 out of 8 PRs are fully finished, you can merge them into trunk. Probably doesn't work always, but can be a good rule of thumb to make the PRs atomic in the sense that if you merge the bottom N PRs of a given stack, trunk will be fully stable. From what you're writing it seems that the reviewers are trying to look at multiple PRs at once to be able to review the feature in full. If you can separate the feature into independent bits (that all can individually land in trunk without stuff added in upstack PRs), then that's a good use of PRs. If you can't, then maybe it's better to make a bigger PR. One issue Graphite tries to solve, that I'm too painfully familiar with, is wall-of-text PRs that are just hard to review or even hard to find time to review, that then end up blocking your devs. If your reviewers preferred how things worked before Graphite, then I think you should simply put more stuff into PRs to make them more "whole feature".
One stack can even have near-unrelated changes that simply have some dependencies to the downstack code. So I think you shouldn't think about it as trying to view a full stack's diff. So it might be a good idea to make your devs created fewer PRs. If they like how PRs separate individual changes, you can simply
gt modify --commit
to push new changes to an extra commit in the PR. Then you get the ability to browse whole PRs, look at individual commits' diff, and should be able to merge individual PRs confidently without caring about what's upstack of them — as long as the PRs contain all the relevant changes they should contain.
TLDR tell your devs to slow down with PRs and make sure your features (feature as in something you can confidently merge into trunk on its own) aren't spread across multiple PRs excessively 😛
c
Hi @Timo Sand! Thanks for your feedback (and thank you @Samuel Stancl for taking this question). We’d be happy to do an onboarding session for your team to talk around best practices for stacking if that would be helpful! Feel free to chose some time on my calendar here 🙂