DEV Community

Discussion on: The Problem with Feature Branches

Collapse
 
michaelmangial1 profile image
Michael Mangialardi

If I'm understanding this correctly, trunk-based development means no code reviews. That is unacceptable as professional developers. Am I missing something?

Collapse
 
bentorvo profile image
Ben Brazier

Trunk based development doesn't mean no code reviews. Code review can either happen at the time of writing through pair programming or after deploying and testing the code in a development environment. Normally it makes more sense to review just before going into production as part of the release step instead of before the code has even been properly tested.

If you haven't deployed and tested it then the code being reviewed might not even work which is a waste of time for everyone. If it is just reviewing formatting then it should be automated and not require human intervention.

Collapse
 
michaelmangial1 profile image
Michael Mangialardi

This presumes that testing the code in a development environment is the same as a code review. Or, to put it another way, a code review just checks that code works and is formatted to correctly.

I'd argue that code reviews are much more than that.

Code reviews:

1) Check not only that the code works, but that the code has the proper threshold of quality, following existing design patterns, clean code standards, and refactoring patterns that ensure that the code is not overly complex and is maintainable and consistent.

2) Work as a means of transferring knowledge and collaborating.

3) Work as a means of documenting the "story" of how the code reached its final state.

4) Work as a means of aggregating documentation. I have often taken longer pull request comments and extracted them into documentation.

Sure. Automate using code formatting and linting tools to shorten the scope of code reviews--but not everything can be covered by automation.

As for pair programming, it is a good exercise but what about other developers who may want to review the code who didn't pair on it? What about when you want to go back and see the historical context of a change? What if both people pairing want to review the code in GitHub before merging, to be sure?

Trunk-based development seems like a case of "the cure is worse than the disease." Just as there are ways to still code review if committing straight to main, there are ways to work out the kinks of feature branches.

Thread Thread
 
bentorvo profile image
Ben Brazier

I'm not saying tests are review. I'm saying that you do review after deploying and testing. Normally this just looks like a diff of the two commits rendered in the pipeline showing the changes next to the button that deploys to the next environment.

Thread Thread
 
michaelmangial1 profile image
Michael Mangialardi

Is there a tool for doing this sort of review? This would seem to break the ability to use GitHub and all of its capability in exchange for resolving some mild occurrences with feature branches.

Thread Thread
 
bentorvo profile image
Ben Brazier

The diff functionality to review changes is built into git. If you want the GUI stuff then just get your pipeline to send a link to the diff on GitHub (docs.github.com/en/pull-requests/c...) you can put the two commits in the url.