(Or rather, I would probably just run on my private git copy, and only pull every once in a while, and ignore that the main branch always changes.)
When / how do you do code review in your suggested workflow?
- the code exactly as it is in that branch
- that branch merged into current main
- that branch merged into what is currently running in production (in the case that it's not what is in main e.g. tagged or branched)
This really helps to see whether it's an issue in "your" code, or whether it's a merge issue conflict causing failures.
See https://bors.tech/ for a similar idea.
What I would object to is changing the 'official' master every few minutes automatically.
Basically, first you write your code however you see fit. Then you use git to rewrite history to make the reviewers life easy, and then you give it to the reviewer.
The reviewer doesn't need to know that my original version had a bug that I fixed later. I can make it look like I came up with a bug free version from the start.
In general I find my past efforts to maintain a clean git history were probably not that useful, as long as you're not running e.g. 4 or 5 branches in parallel with crossing history. Branch off, make change, merge back main, PR, is fine.
Sometimes it makes sense to have multiple commits. The next step up the complexity ladder is:
- one preparatory refactoring commit that does _not_ change behaviour
- one simple commit that changes behaviour
Or are all the merge commits just made provisionally to inform you, but don't change master?
Your suggestion seems to help with the former, but not really the latter, or does it?
You might like https://bors.tech/ however.
Not in my experience, no.
The goal is just to keep you aware of what problems you're likely to run into if you tried to push for your change to be merged in at the present moment. If these problems are due to work another team is doing the idea is then that you might begin conversations with that other team to discuss how best to sort these things out.
This trades the integration complexity and problems for some new.. challenges :)
* everybody is good at breaking down code into changes small enough review quickly but not small enough to become tedious and trivial
* people work on CRs whenever they have time. They do generally not post them after people have started going offline, and wait til next morning as a courtesy, since there is no difference between publishing for review at 5pm vs 9am the next day
One side effect of this workflow is that because the pieces are more manageable, less uninterrupted “flow” time is required to complete a bunch of small things than to make one really big change. And others digest the smaller changes easier and knowledge spreads more effectively. And with the time its easy to say “don’t make people review outside working hours.”
My team does stuff on mainline and requires a new review on rebase.
https://www.atlassian.com/continuous-delivery/continuous-int...
Main caveat is that “prod” is a handful of central places.
So how does code review fit in?
- The first thing is that your main branch needs to be able to tolerate merging incomplete code without breakage. This would probably be resolved with feature toggles, ideally named to match issue/ticket numbers, so that you have to clean it up before closing the ticket. Automatic tests can catch syntax issues but code review would check that your code was indeed either removing a feature toggle (in which case the team should have agreed that the code is mature and turned it on in production) or properly isolated behind a toggle.
- The second thing is that the team needs to be working as a team. This means that the sprint goals are co-owned by multiple people, ideally the whole team takes responsibility to deliver. It never made much sense to me that we segmented out features to individual developers, as if “oh she is out sick this week, well she wasn't working on anything urgent, the work can wait for her,” I understand that this makes performance review slightly easier but I am not convinced by that argument... So what this means for development is that the “war room” that you see for a typical prod outage with a bunch of people chasing down different leads, I want to bring that mentality (without the concomitant stress!) to feature development. So this means that ideally the whole team knows what everyone is working on and code reviews are just part of the organic process of working on a feature together. «Hey you are calling this “accept_states” and I called it “success_states,” I really don't like the fact that you're using “accept” as an imperative verb here, but I could go with “states_to_accept” or “acceptable_states” or we could use “success_states,” which would you rather?» ... You read through each other's code because it legitimately impacts what you are doing elsewhere in the codebase.
- The final ingredient would be release staging. The way a lot of people do branching makes it really easy to forget commits or to roll something back and not rollback the rollback, et cetera. Because we merge everything into main, we get monotonic flow: a commit that is still relevant is merged into main first because everything is, then it gets cherry-picked into the appropriate release branches. (The only exceptions are bugfixing code that is no longer part of the latest version.) The exact tooling to make this doable seems tractable, but perhaps not easy. Per point 1, before the feature branch is permanently flipped, all the code related to it is marked as such and can be merged into a patch release without actually triggering a semver violation (just leave the toggle turned off!)... So in theory the actual feature release is just a single commit. I’d have to see how the rest of this works before I would know exactly how this plays out.
Note that just about every Git-based CI/CD platform does CI/CD incorrectly, things like ACLs and CI/CD config are central statements about who has access to what privileges and should not be branched. A Jenkins which pulls its config from the `ci` or `main` branches will usually save you a lot of headache.