# Please slow down the code review process!
**URL:** https://discourse.itk.org/t/please-slow-down-the-code-review-process/631
**Category:** Community
**Created:** [January 30, 2018, 12:50pm UTC](https://discourse.itk.org/t/please-slow-down-the-code-review-process/631 "2018-01-30T12:50:53Z")
**Posts on this page:** 9
**Page:** 1
### Author:  [@Niels\_Dekker](https://discourse.itk.org/u/Niels_Dekker)
#### Post date: [January 30, 2018, 12:50pm UTC](https://discourse.itk.org/t/please-slow-down-the-code-review-process/631/1 "2018-01-30T12:50:53Z")
Dear ITK-ers,
I feel that the code review process is going too fast. Sometimes I see patches being submitted and merged within the very same weekend. Now I just reviewed a patch for which I received an invitation yesterday evening, but I’m too late already: It has already been merged. ☹
Do you expect reviewers to be available 24/7?
Otherwise, wouldn’t it be reasonable to give reviewers a little bit more time, for example by:
- avoiding merges to the master during the weekend
- allowing 24 hours between the final patch set and the merge to the master
?
---
### Author:  [@fbudin](https://discourse.itk.org/u/fbudin)
#### Post date: [January 30, 2018, 2:55pm UTC](https://discourse.itk.org/t/please-slow-down-the-code-review-process/631/2 "2018-01-30T14:55:59Z")
I would just like to add that this is especially the case for feature patches. Patches that correct bugs and broken dashboards, in certain cases, need to be merged fast.
---
### Author:  [@matt.mccormick](https://discourse.itk.org/u/matt.mccormick)
#### Post date: [January 30, 2018, 3:24pm UTC](https://discourse.itk.org/t/please-slow-down-the-code-review-process/631/3 "2018-01-30T15:24:00Z")
Thanks for discussing this, @Niels_Dekker @fbudin.
Enabling code review helps improve our process.
@Niels_Dekker did you want to submit a pull request to [the ITK Software Guide](https://github.com/InsightSoftwareConsortium/ITKSoftwareGuide) repository? A note could be added to the [Software Process](https://itk.org/ITKSoftwareGuide/html/Book1/ITKSoftwareGuide-Book1ch10.html#x49-16300010) chapter stating that proposed patched should not be merged until one business day has passed to allow for code reviews. An exception to this rule would be patches that clean up the dashboard or address critical bugs.
---
### Author:  [@seanm](https://discourse.itk.org/u/seanm)
#### Post date: [January 30, 2018, 5:08pm UTC](https://discourse.itk.org/t/please-slow-down-the-code-review-process/631/4 "2018-01-30T17:08:08Z")
Niels,
I have noticed this too, and am in complete agreement.
Sean
---
### Author:  [@Niels\_Dekker](https://discourse.itk.org/u/Niels_Dekker)
#### Post date: [January 30, 2018, 5:59pm UTC](https://discourse.itk.org/t/please-slow-down-the-code-review-process/631/5 "2018-01-30T17:59:13Z")
I understand well that some critical bug fixes cannot wait for 24 hours. But do you think that a patch that would only fix a rather harmless compiler warning (thereby cleaning up the dashboard) should also get a shorter review time?
I would prefer to only have a shorter review time for critical bug fixes and compilation error fixes.
Of course, critical bug fixes do often need a careful review anyway.
---
### Author:  [@dzenanz](https://discourse.itk.org/u/dzenanz)
#### Post date: [January 30, 2018, 7:11pm UTC](https://discourse.itk.org/t/please-slow-down-the-code-review-process/631/6 "2018-01-30T19:11:52Z")
Most people work on ITK on the workdays. But some (@hjmjohnson?) seem to be working on the weekends.
I think that asking for a whole workday to pass before non-critical patches are merged is reasonable. Perhaps this condition could be waived if multiple people review the patch quickly?
---
### Author:  [@Niels\_Dekker](https://discourse.itk.org/u/Niels_Dekker)
#### Post date: [January 31, 2018, 2:17pm UTC](https://discourse.itk.org/t/please-slow-down-the-code-review-process/631/7 "2018-01-31T14:17:45Z")
@dzenanz It’s fine if people like to work in the weekend, of course. But others may have a party on Saturday and a hangover on Sunday 😀 Those ones should still be allowed to review during workdays, right?
Even if multiple people have reviewed a patch quickly, someone who is _explicitly invited_ to review the patch should get a fair chance to do a more thorough review, in my opinion.
It would be nice if Gerrit would have a “Slow Down” button, to be pressed by (potential) reviewers. Or if Gerrit would provide an indication how much time is left for reviewing. (If you assume, for example, 24 hours review time by default, it would not be too hard to implement.)
Of course, there should always be room for exceptions to the rules, but I think some guidelines would be helpful.
---
### Author:  [@jhlegarreta](https://discourse.itk.org/u/jhlegarreta)
#### Post date: [January 31, 2018, 4:22pm UTC](https://discourse.itk.org/t/please-slow-down-the-code-review-process/631/8 "2018-01-31T16:22:20Z")
Having some policy that we agree upon may be sensible.
Some suggestions, such as relying on the type of topic/patch, are interesting to accommodate the process.
Having a deadline or “slow down” button, requiring the approval of some maintainers, or a number of people, etc. seem desirable also.
But implementing these in `GitHub` (remember that [ITK is transitioning to GitHub](https://discourse.itk.org/t/gearing-up-for-itk5/515)) or requiring the `GitHub` people do it may not be easy/out of reach.
Having said that, the truth is that:
- Current **maintainers are mostly based in the US** , and accommodating different time zones may not be that easy, and workdays may also differ across countries, across companies, etc.
- We should acknowledge that maintainers’ institutions may also have **other constraints** (funding agencies, etc.) that one, including me, as a community member, may be unaware of.
- People having push access are trustworthy and work to the best of their possibilities, including outside work hours, to make **ITK better for the community** 👌, and the funding agencies, institutions, scientific community. Even if this means merging within short delays.
- A good feature, module, class or awesome contribution in terms of bug fixes can beat missing a review. **So there is always a chance to improve ITK**!!
Just a short explanation @Niels_Dekker: the `gerrit` review system **automatically** adds people as reviewers when it detects that a given user made some commit on the file at issue at some point in history.
---
### Author:  [@blowekamp](https://discourse.itk.org/u/blowekamp)
#### Post date: [February 2, 2018, 1:23pm UTC](https://discourse.itk.org/t/please-slow-down-the-code-review-process/631/9 "2018-02-02T13:23:59Z")
While it is all good to have an official policy with this stuff. In many ways just common sense needs to be applied.
For example I presume if I get a +2 on [this patch](http://review.source.kitware.com/#/c/23141/) I can merge this “trivial” fix without allowing the prescribed full business day for review.