The boy scout rule and git in practice

There’s a dichotomy when applying the boy scout rule to programming: cleaning up code that you happen to come across ‘pollutes’ your merge-/pull-requests, making it harder to review and therefor more unlikely to be accepted.

One way to cope with this is to submit the ‘clean up’ and the feature/task related changes separately, and merge them back into upstream in separate steps. But often times, it is much easier to just fix a small problem right away instead of switching back to your main branch and doing it there. In fact, it might prevent the developer from doing the improvement, which I want to avoid. Quite the opposite, I want to encourage my fellow developers to do improvements.

So one thing that we do about this is to mark the changes that are unrelated (or tangentially related) to the task with their own commit and a special prefix in the commit message like:

BSR: More consistent function signatures

As you might have guessed, BSR stands for boy scout rule. This does not solve the fact that the diffs get larger than necessary, but it makes it possible to ‘filter out’ the pure refactorings. In some cases, these commits can later be cherry-picked onto the main branch before doing the review. Of course, this only works for small refactorings, but this is where the boy scout rule applies.

2 thoughts on “The boy scout rule and git in practice”

  1. Having recently looked at hundreds of commits from a lot of different top rated github projects and ranked their quality along 5 dimensions, one of which was commit atomicity / SRP (see e.g. https://who-t.blogspot.com/2009/12/on-commit-messages.html or https://link.springer.com/chapter/10.1007%2F978-3-319-92375-8_6) showed me how often boyscouting is done, but also how difficult it is to decide whether it really is still boyscouting in the sense of a minor cleanup of code you came across.

    Due to this difficulty and the dichotomy you mentioned, i.e. the false incentive of omitting the fix if an automatic reviewer would penalize boyscouting, we dropped that dimension, but it was a hard decision.

    If there were a way to do boyscouting without much git overhead, I would actually prefer to keep the dimension. But I fail to see how creating an own commit on your current branch, or picking the boyscouting hunks later on into an own commit, is any less annoying than noting down the cleanup you want to perform and doing it later in an own pull request / merge request. Could you please elaborate and explain your git process (stashing? interactive staging?) from the moment you come across the code you want to clean up?

    1. Once I come across code I want to clean up, I immediately commit before changing anything. Then clean up what I found and commit again with the BSR tag.
      Sometimes I mess up and the BSR part remains in the same commit.
      I decide how to handle those commits during review. They can either be extracted with or without tool help (e.g. cherry picking or manual separation) to be merged before merging the feature. If the change is small enough, we just leave it as is. This is up to the reviewer.
      I personally stay away from stashing or interactive staging to keep the process light-weight, but they are not actively discouraged.

Leave a Reply to DavidFarago Cancel reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s

This site uses Akismet to reduce spam. Learn how your comment data is processed.