Who should do code reviews on Sass changes?


#1

For example:

Is this mainly just for visual design @mray to review or do we want anyone else (including potentially a role not yet filled) to review such changes? Is it appropriate for @singpolyma as a reviewer?


#2

For content/functionality (“Does this look/behave right?”), mostly just @mray/@msiep.

Personally, I’d appreciate a separate review of my css changes in two ways:

  1. Is this the best way to do it?
    • I don’t know much about css idioms, so I’d appreciate someone checking that I’m not using padding when I should be using margins, or something like that.
    • Anyone with a bit of css experience could do this.
  2. Is this the right place to put this?
    • Ensuring that our code is consistent, and that I’m not rewriting something that already exists in the code base that I don’t know about.
    • This should be someone familiar with the Snowdrift css in particular. Maybe @iko would be up for it as a lower time commitment way to stay involved?

#3

I don’t feel sufficiently close to the codebase to review sass code changes. I can imagine chipping in my feedback based on my limited CSS knowledge, though.


#4

Ah, my comment was poorly worded. Edited now.