DEV Community

Discussion on: The Pull Request Checklist

Collapse
 
mkrl profile image
Mikhail Korolev • Edited

Have you considered running any kind of CI/CD service? That's basically almost everything you mentioned. There are different approaches for almost every language out there.

Enforce different rules through code linters, practice checkers and tests. Enforce a rule of never decreasing code coverage. Only merge PR after everything passes.

Ideally, your development process should look something like this (I have no idea what is your stack, let's take a small open-source Rails project as an example):

  1. Make local changes you need
  2. Before you commit anything, run tests. rake test
    • rspec: generic unit tests, ensuring everything still works
    • Rubocop: complex code linter. Do you want your code to follow a certain style? Describe how you want it to look like. Limit line and method lengths, check for global variables, force to wrap reusable code into separate code blocks.
    • Anything else you want, there are a plenty of gems and different solutions.
    • Make sure the coverage at least remains the same.
  3. Commit if everything passes
  4. Push to remote, make a PR
  5. During the PR process, a CI service, let's say Travis picks your commit up, runs all the tests and marks your commit with a nice green checkmark.
  6. Merge into master without worrying. Plus, this can be automated if needed.

Running this cycle even with smaller projects can greatly benefit your code style. If you do it on a regular basis, the "oh my god, another check failed again" will turn into writing code that is initially clean and easy to maintain.

Collapse
 
idoshamun profile image
Ido Shamun

We are indeed working with CI/CD and following the same best practices as you mentioned. The checklist is meant for the reviewer as a guideline for his/her review. Even with massive automation tools code review is still needed