Showing intent with mixin overrides

I write a lot of mixins in my SCSS. It makes managing large component based websites with strict consistency requirements possible.

Without them it's all too easy for a developer to overlook specific nuance that leads quickly maintenance issues. When a minor, but common pattern needs to be updated and isn’t available in one spot? "Hope you didn’t forget that one margin-top you used in 100 places that is now 5px smaller."

As with any abstraction writing mixin heavy SCSS comes with it’s own set of difficulties. Recently I've put some time into finding a clear and concise way to override specific mixin rules when a property doesn’t deserve to be parameterized.

Let’s say you have a standard subtitle mixin that you use through a site to define a set of typographical rules that should be common for various semantically similar elements.

@mixin subtitle {
  @include font(open-sans, semibold);
  @include font-sizes($header-font-sizes);
  margin:0 0 1em;
  color:$dark-grey;
}

What often happens deep into a project is that you find an edge case where, for a design reason, the standard subtitle margin of 1em doesn’t work. This change does not warrant it’s own parameter. The clutter and restriction a new parameter places on the mixin is not worth it. So you’d probably do this:

.special-situation {
  @include subtitle;
  margin-bottom:2em;
}

Great. It’s done. You can see that the special-situation is a a subtitle and it’s bottom margin is 2em. What this doesn’t tell you is if that margin is specific to this element or a change to the mixin. Throw in a few more mixin and try to find the intent the developer had.

.special-situation {
  @include subtitle;
  @include gutters;
  display:inline-block;
  max-width:50%;
  margin-bottom:2em;
  color:$blue;
}

It’s nearly impossible to tell why the margin has been set. You can’t know that one of the rules is an override and even if you guessed… which rule is it overriding?

Now. Don’t even consider breaking your declaration ordering style rules (you have those right?) by moving the rule below @include subtitle;. I’ve found something much better.

To solve this problem I'm using the @content control. This feature allows users of your mixing to attach a trailing block to any mixin inclusion that can be used to clearly relate overriding rules to the mixin. In the example above you’d have:

.special-situation {
  @include subtitle {
    margin-bottom:2em;
    color:$blue;
  }
  @include gutters;
  display:inline-block;
  max-width:50%;
}

It’s now completely transparent to any reader of this rule that the margin is 2em, and is overriding a rule within the subtitle!

Our design requirement is met, our mixin is clean of overly specific parameterization, and our rule ordering is correct! But more importantly than all of that, is the developers intent is clear and obvious during any maintenance on the styles in the future.

I'm blocked by a code review again and it's your fault

At Meerkats we peer review every single piece of code that is written, before it can be merged into a build. With how busy we are, that can lead to blockages that can be easily defended against.

We use Github's Pull Request system extensively for our code reviews. All code written in the agency is put into a new branch (we kind of use git flow) that will eventually be ready for sign off from a peer. To be reviewed a branch is pushed to Github and a new PR is opened with your changes and some documentation (in the form of a title and a description) to ready your code for another developer to eye over.

That developer (or a few) will go through the code, make comments and suggestions or ask a few questions about some awesome piece of code. There may be some changes required, so the original developer will write some code, push it to the same branch (which updates the PR) and ask for a follow up review. This goes in circles until it's been signed off and the branch is merged.

As we've grown it's not uncommon for us to have 20 or more reviews that need attention on any given morning. It became evident that we were going to need some sort of system to keep track of each PR's current state to ensure we're not wasting time on reviewed code or missing a crucial bug fixe in our builds because it was missed in review.

So we started using labels. We don't use Github for issue tracking so the defaults were sitting there not being used for anything in particular. Our initial requirements were easy:

  • ready-for-review
  • awaiting-followup
  • blocked
  • help
  • question
  • shipit

The last person to touch a PR (either the developer after they push some code or a reviewer after a comment) is responsible for updating the PR's labels to ensure that the state is clear. Now when I sip my coffee in the morning I can see a long list of all our outstanding code and know immediately which need my attention as a developer or as a reviewer.

This was working great, except for that our set of labels was not in sync with the default set that Github provide. Every time we open a new project (this happens very often in a digital agency) someone (me) is forced to manually replace all the default labels with our own set ensuring consistent colors. This was a major slog each time (especially as our labels have grown) so I got fed up and wrote a command line tool for this.

https://github.com/jimmyhillis/labels

It's command line program written in golang (my first attempt!) that allows anyone with access to a repo (via a token) to easily add and remove labels from a repo when initialising a project. So any time I open up a new project I need only run two commands and we are ready to roll.

$ labels remove jimmyhillis/labels
$ labels add jimmyhillis/labels ready-for-review:fbca04 awaiting-followup:bfd4f2 shipit:207de5

It's very much a work in progress and I'm already planing some improvements to the tool which hook into hub to help automate setting the state of a PR after important actions.