A
A
Alexander2016-04-25 22:34:49
Programming
Alexander, 2016-04-25 22:34:49

What is the correct workflow for code review?

What is the correct workflow for code review?
For example, we have a "dev" branch in our git repository, which is being developed, and I see the following options:
1. Tasks are developed in separate branches and merged to "dev" only after passing code review.
2. Tasks are developed in separate branches and merged to "dev" immediately, without review. But, in addition, another branch is created (say, "review"), into which the same tasks are merged, only after passing the code review. The "dev" and "review" branches are periodically synchronized.
In option No. 1, I see a minus in that the deployment of tasks is very slow. In fact, until a person is released and does not review, the task may not get into the "dev" branch for a very long time.
In option number 2, it seems to me, there is a high probability of conflicts due to the fact that you have to constantly synchronize two branches.
How are you doing?

Answer the question

In order to leave comments, you need to log in

3 answer(s)
V
Vit, 2016-04-25
@fornit1917

Without review, we only merge hot fixes.
For the rest of the tasks, we make pull requests, which must be reviewed.

L
leclecovich, 2016-04-26
@leclecovich

The team set up the Code Review system to prohibit commits to develop/master, only merge from the branch. To merge, you need at least one approve of someone from the team and a successful run of unit tests. For the latter, integration with CI was configured.
As a result, the quality of the code has increased, the tests are always up to date. This approach allows you to find a large number of stupid mistakes even at the review stage. Oddly enough, not much time is spent on review - about 5% - 10% of the total task completion time.

S
samizdam, 2016-04-26
@samizdam

Why I recommend doing a pre-merge review (option 1):
1. psychologically: it’s easier to return the task to the developer and make changes to the code before the feature branch goes into the main one
2. technically: from the point of view of testing and conflicts, it’s also easier to make changes to the topic branch before merging rather than after to the main one
Let, if you trust the developers and do not have time to review all pull requests - then give the developers the opportunity to merge themselves, and in the interface (gitlab / github / bitbucket) you can always review a closed merge -request, post-factum. Just accept the convention that all merging to the main branch is done through this one interface.

Didn't find what you were looking for?

Ask your question

Ask a Question

731 491 924 answers to any question