[1054] Add violated violations to notes

[squidgetx]

Resolves #1054

[orenyk]

Ha, I was looking at your first commit and thought, "I wonder if you'll end up with a nil error when there are no notes" :-P. Glad to see I'm developing some intuition :-)

[orenyk]

This looks good, just merge master back in and we can pull it in.

[mnquintana]

@orenyk I can't quite remember now – is there a reason we've been preferring merges over rebases?

[orenyk]

@squidgetx since we use remote branches, rebasing would require 'rewriting history' on origin and a git push -f to send changes to GitHub (post-rebase). Rebasing would provide a cleaner commit history, but IMHO it's better to avoid force pushes whenever possible.

[mnquintana]

@orenyk Ah okay. I know the theoretical reasoning for this, but have we run into issues with force pushing to GitHub?

I've gone back and forth between the "preserve commit history exactly as it happened" and the "make the commit history actually useful and reflect intent" camps a lot, but now I'm pretty much in the latter. If a group of commits are logically related (which they should be in just about every PR), they should be rebased and squashed into one commit for:

1. easy CHANGELOG writing
2. easy reverting
3. a more readable and readily understood dev history

AFAIK, we'd only run into rebase problems if someone branched off the un-rebased branch before force pushing (side note: did they intentionally make that a Star Wars reference? 😝 ). But how often is that ever going to happen? This would be a massive issue on master, because it's the canonical development branch and everyone is always branching off of it, but branching off a feature or bugfix branch is a lot more rare.

Take #1078 and #1076, for example. I branched off of @orenyk's feature branch, and then made a PR to #1076. (Incidentally, I squashed all my commits before doing that.) Rather than merge in extra changes and add more noise to the commit history, I rebased off your branch and force pushed changes to mine, because I knew no one was branching off my branch. Only once #1076 was merged would you then squash commits in #1078 – you only rebase and squash right when it's ready to merge into master.

(This discussion is also extremely relevant for #1056 – we'll need to decide if we want contributors to squash commits or not)

[orenyk]

I know... I really do. The honest truth is that I was much less familiar with Git back when we started establishing some kind of precedent, so I feel much less strongly about force pushing than I did back then. On the one hand, when you're expecting to be working with people less familiar with git / people with less experience (as has been the case in the past), avoiding force pushing at all costs makes way more sense. Now that we're transitioning to a wider audience, and can reasonably expect our contributors to have a little more experience (and can also reasonably hold them to a higher standard), moving over to rebases / squashed commits probably makes more sense.

I was going to hash this over again when I got into writing #1056, but I'm definitely open to switching over to a rebase model if we think that it won't overcomplicate things for our new devs next semester (v4.3.0 sprint).