All code is a work in progress and every PR is a request for comment. Since our tools should be used to help us work, we first should decide how we want to work.
- Always merge your own PR, and never merge someone else's PR.
This is your code and you're the expert at how it should be merged and how conflicts would be resolved. If it's understood that you're responsible for your own code until the point it's merged, then you won't need to worry about accidental merges. For people who merge other people's code: stop micromanaging.
- Submitting a PR should be the one and only standard for requesting feedback.
Each reviewer has their own standards and as a team you'll have common standards. There is no reason to ask for different standards when requesting a review, regardless if the code will be merged or it's just an idea. Adopting this rule also means your team knows the role of a PR and has a direction to which to improve the process. Common additions to the PR is automated testing, syntax rules, security checks, etc.
- You should indicate in code the intentions (if possible)
In general all PRs should be ready to be merged at the time the PR is made, that's the idea all the reviewers should have when reviewing. Whenever possible, you should make features such that it can be controlled with a feature flag. Even if you're not ready to merge, or don't have the intention, your code should be. Designing features that can be controlled with flags encourages modular designs, allows the code to be merged in small pieces, and most importantly indicates this work is a Work In Progress.
- Submit PRs along the way
As you build a feature you'll find your PR is made up of code needed to support the feature and the code for the feature itself. Support code would be refactors required, creating new utilities, adding a new service, etc. While this is needed for your PR's main feature to work, it could also be done as its own PR. This allows you to get feedback early while also being able to merge the code, in addition you can continue to work on feature while waiting for comments on the smaller ideas. When it comes to the main feature, there should be far less feedback to even request.
- All code is work in progress
All code merged in should be near perfect, but with the understanding that you can always make another PR. The first priority of a PR is to ensure the code won't break the application, so that should be the main purpose of the review. The second priority is to ensure the code can be maintained, like style and common practices. Anything beyond the strictest measure of correctness and strictest definition of maintenance is extra, and should be understood as such. Always feel free to make any comment, but also have the goal that the code should be merged in ASAP. But don't create TODO tickets just to get a PR merged, that's purely unethical.
- The PR process is also a work in progress
Teams of different sizes and capabilities will have different practices. As your team grows and evolves, make sure you allow your process to grow and evolve too.
git diff first-branch second-branch
. Very complicated indeed. Devs need their time for more important tasks than learning a half-liner of the most important program in their career (IMHO git is more important that gcc, by a slight margin).