Changes Requested
Summary
Whatever your personal philosophy may be, the team as a whole and often the company has to come to some sort of consensus as to what constitutes a reason to block a PR. At an organizational level, blocking PRs is a stronger guard against not letting bugs, technical debt, and security issues into your app and maintaining stability. On a team level, however, there are several reasons that requesting changes is helpful for communication as opposed to reviewing and not giving approval. I especially love the focus on how you need to highlight the positives in PRs, and this becomes even more critical on a team to foster goodwill if you do reach for that blocking button. I created a mega list with some help from fellow software engineers at C5 of reasons that developers may or may not want to reject a PR from the super obvious “fails to compile”, meaning it would entirely break the app, to the more subjective “non-descriptive naming”.