DEV Community

Discussion on: What's your typical process for reviewing a pull request in GitHub?

Collapse
 
szymach profile image
Piotr Szymaszek

I would advise against merging code with bugs, which can end up in production pretty easily if you are not careful.

Collapse
 
isaacdlyman profile image
Isaac Lyman

That's a great point, and to clarify, I wouldn't usually approve a PR with bugs. But I think there are cases where it's acceptable:

  1. The only bugs are design inconsistencies or unclear requirements (in larger organizations, these can take weeks to resolve).
  2. We're doing new feature development, the PR is a major net improvement over what we had before, and the bugs are minor and part of the new feature (not regressions).
  3. The team agrees that the PR is large enough and affects enough things that we should merge, deploy it to the company's internal server, and get a lot of eyes on it as quickly as possible. This is an intentional delay of all other work until the code in question is thoroughly tested.

Every day that a PR sits unresolved is technical debt; it gets further and further out of sync with the master branch, the original author starts to forget what they did, and all the people that could have been using and testing the new code, aren't. So the risk of letting bugs into production isn't the only risk in this situation, nor, I submit, is it always the greatest risk.