NERD Summit - March 18, 2023
Michael Lynch (mtlynch.io)
Which is easier?
Which is easier?
Example bad changelist description
Move user.js
This change moves user.js to the src/controllers/auth directory.
Example good changelist description
Move user.js to src/controllers/auth
When we created user.js, most of its callers lived in src/lib/user.
We did a lot of restructing as part of the 4.2.x release, so now all of user.js’s clients live in src/controllers/auth, so this change moves user.js to be closer to the majority of its clients.
Changelist | Review difficulty |
---|---|
One-line change | Easy |
Whitespace-only change | Easy |
One-line functional change within 200 lines of whitespace changes | Painful |
What happens next?
There are two ways of constructing a software design. One way is to make it so simple that there are obviously no deficiencies. And the other way is to make it so complicated that there are no obvious deficiencies.
-Tony Hoare
Reviewer: This function is confusing.
Example bad response (defensive)
Reviewer: This function is confusing.
Author: What, exactly, is confusing about it?
Example good response (agreeable)
Reviewer: This function is confusing.
Author: What changes would be helpful?
Reviewer: I think this 8-line function would be better as two 5-line functions.
Author: I think it’s clearer as a single function.
Who’s right?