department-of-veterans-affairs / va.gov-team

Public resources for building on and in support of VA.gov. Visit complete Knowledge Hub:
https://depo-platform-documentation.scrollhelp.site/index.html
283 stars 204 forks source link

Automate "Best Practices" Enforcement on Pull Requests #1934

Closed omgitsbillryan closed 5 years ago

omgitsbillryan commented 5 years ago

The Problem

Some common mistakes are made when starting pull requests:

  1. The PR is too long
  2. The PR changes code in /db/migration & /app at the same time (changes to /db/migration should be done in a standalone PR)
  3. (Any more ideas?)

Work to Be Done

kfrz commented 5 years ago

This is awesome. :) :pray:

<soapbox>


I've ranted on this before but I would love to see us improve our overall commit message quality. A quick git log -a shows tenses interchanging, un-squashed messages w/ meaningless information, and variant strategies for writing said messages.

I think the value in maintaining a highly-readable git log acts more like an insurance policy. This is hand-in-hand with making atomic commits that can be rolled back or cherry picked if needed. We hopefully don't need to spelunkdebug via git log all too often, but it's made easier by taking a bit of care when crafting the message.

When in doubt, consult the master himself, Linus. These commit messages are the pinnacle of excellence, and we should aim for excellence in all things.


</soapbox>

omgitsbillryan commented 5 years ago

git-cop requires Ruby 2.6 or higher 😢 . fit-commit seems like a reasonable alternative that does pretty much the same thing, albeit slightly lesser adopted. this would be a nice addition IMO.

master is configured as a protected branch, which I'm pretty sure means that commits directly to it are not allowed. vets-api is also configured to disallow merge and rebase commits to master - it will only allow squash commits.

omgitsbillryan commented 5 years ago

No longer blocked since our jenkins jobs are now "Pull Request" discovered.