3 ms·
That's not what they're saying. Srs. should still review jr. PRs. They're arguing that Jrs should also review Sr. PRs
by apengwin 6y ago
That's not what they're saying. Srs. should still review jr. PRs. They're arguing that Jrs should also review Sr. PRs
- eithed 6y agoWell, if you'll allow everybody to review everybody PRs then eventually that's what you'll get. Similarly - junior might look at your PR and see that it's ok. Senior might look at it, see that you've forgot about a thing that junior didn't know about, you omitted to test a certain condition, etc. Again, juniors are more than welcome to look at PRs and be a part of discussion, but if you're merging, you should be wary. I'd personally be against allowing a merge if you get only a junior approval.
- lowbloodsugar 6y agoThe difference is "approve" vs "criteria for merging". We have a system that requires two approvals, but one must be from a list of seniors (which doesn't include new hires).
- eithed 6y agoMhm - if you'll separate your review process from approval process, then yes.
- CodeMage 6y ago> Well, if you'll allow everybody to review everybody PRs then eventually that's what you'll get. If you allow everybody to review every PR, it doesn't imply that you'll allow everybody to approve every PR.