151–160 of 230 posts
I am one of the authors of the tool Graphite ( https://graphite.dev ). We built Graphite because we missed the stack diff workflow we knew from previous job experiences at larger companies, happy to answer any questions (and if you would rather not ask in public, feel free to email me at tomas at graphite.dev).
It depends on the project being worked on. If the main branch changes often enough that you need to merge master before you merge the branch you’ll spend a lot of time propagating changes. And then there’s the problem of large codebases . Without a really good cache strategy for builds all of the branch switching and merging will cause a lot of rebuilds. For some languages and large projects that can be 20+ minutes f…
Earlier quoted context omitted.
It’s unavoidable sometimes. I get inspiration and time together rarely, I can’t wait for small chunks of code to be merged before I continue. A lot of times it’s an extremely Productive Sunday afternoon and I have 2500 new lines of code that builds a full new prototype. What am I to do?
I understand that (experienced the same "problem" today), but writing "2500 new lines of code" on a Sunday afternoon is (hopefully) not representative of regular workplace conditions.
I've usually kept a rule that you should avoid stacking, and if you must only one level deep. The fact that you have to stack in the first place typically suggests that PRs aren't being merged fast enough. Stacking in my personal experience usually leads to merge conflict hell as changes and PR suggestions get merged underneath you.
This often means first a code cleanup that doesn't change any functionality yet, but makes the later changes simpler - eg; removing dead code, removing unnecessary abstractions/interfaces/layers, upgrading external packages.
Sometimes I recognize this early, and will specifically make a branch for this. If there's changes from the review I'll rebase my next branch(es) on top.
Often I don't see this will be needed until later (or rather: I'm focused on the change itself and not the PR experience) and so I'll interactive rebase to put all the refactor commits first, and make a PR for those.
Both cases mean I already have a branch built off an unmerged PR. Sometimes even before the PR is published, actually. I don't see any particular problem with this. My development style and speed is independent of the speed PRs get merged; the ability to rebase makes this a total non-issue.
Earlier quoted context omitted.
In one of my first jobs, code reviews were done exactly like this, with the the reviewer (my boss) and I sitting together at one computer going through the changes. It definitely has benefits but it's still important to ensure recommendations/concerns etc. get written down and doesn't necessarily help if there's a need to go away and make significant changes based on the outcomes of discussions. But while the back &…
> and doesn't necessarily help if there's a need to go away and make significant changes based on the outcomes of discussions. See, I disagree, because this is absolutely the place where it helps the most—you can now go away and keep working on the same task without having to context switch to anything else or remember where you were or what you were working on. So you're never in a state where you're blocked and can…
Generally we want to reduce any accidentally complexity and the way to achieve that is to reduce the latency of code reaching production. So for example you want to minimise PR/branches in flight ideally to 0. So you should be asking how you can reduce them - eg trunk based development, omitting asynchronous code reviews etc.
Stacked reviews is a very natural pattern if you're using a system like Gerrit, where the unit of review is not a whole branch, but individual commits. Since they are commits, the reviews can have any kind of relationship that commits can have. I think it's a great system, including for some other reasons (it encourages amending and rebasing commits during the review process, which results in a very clean git history…
We use GitHub Enterprise where I work now. I do sometimes do stacked PRs but GHE does not make it easy. The rebasing and merge conflict resolution can be a headache although git rebase —onto helps a lot. I previously worked somewhere that used Phabricator. Its “stacked diffs” worked great. I’d use it all the time when working on complex, multipart changes.
Try out the linked tool git-branchless ( https://github.com/arxanas/git-branchless , I'm the author). It should help you restore Phabricator-like workflows. In particular, check out the `git sync`, `git restack`, and `git move` commands for handling rebasing and conflict resolution.
So that's why you get to: stacked PRs and whether or not they're good.
Rewind to the place where you decided to use PR process in the first place and consider if that was a wise choice.
There is active work related to teaching "git rebase" to natively support stacked branches in the Git core currently being worked on by Derrick Stolee [1]. If you "stack" your changes across multiple inter-dependent branches it looks like "git rebase" is going to learn how to update related branches using a new "update-ref" command (alongside "squash", "fixup", "exec", etc) that gets activated automatically through a…
Earlier quoted context omitted.
This is something I've come to realise as I've matured as a developer. I just don't care what style you write in, if you don't use all the syntactic sugar, etc. I care where you draw your module boundaries, which units depend on which, and if you've handled edge cases, and so on. That's what's going to matter in the end. That's where I'll have to spend a lot of time figuring stuff out. Not syntax.
Exactly. I’ve seen style-perfect lint-free “write once” spaghetti code that drops edge cases and error conditions left and right, and I’ve seen “ugly” but clean code that needs no introduction and works perfectly to boot. I worry that auto-formatters (not to discredit their upsides) do sometimes give people a false sense that they’re writing good code.