Are you an annoying, picky, pedant code reviewer?
I guess I sometimes am.
No, I don´t argue about tabs or semicolons ( we use a linter for that).
And I don´t ask to rewrite code only because I would have written it slightly differently.
Well, I will give you a bit of context:
Out of 9 people in my team, none is a native English speaker.
Can you already imagine what I mean?
In such a working environment - with developers from India, Egypt, Italy, Poland, Russia, China, Bulgaria - with different languages and alphabets - it can happen very often that you drop or find a spelling mistake.
Maybe the code is absolutely fine, nice implementation and unit tests are there, but here you go:
const analiseData = (data)=> //awesome code here
What do you do?
Do you reject the Merge Request ( or Pull Request - if you are using github) just because of that?
Or maybe the MR is already spotted with comments and requests of modifications, do you add typos on top of all that?
Wouldn´t it be nitpicky?
As much as I don´t want to hurt people feelings, to sound like a boring English teacher or simply to slow down our sprint by becoming a bottleneck for all the tickets in Approval for a MR, I really believe typos should be eliminated from the code.
Code must be human-readable and must be written for your coworkers and your future self.
If you allow a method named
loadCampaings to pass the code review and make it into the master code, this could happen:
- other devs not aware of that bad spelling might invoke a missing function or access a property that is not there, leading to bugs.
- or they will have to add some cognitive load and domain project burden having to remember that that function is not called
- if you remember there was such a function and search for it / grep it you won´t be able to find it ( assuming you are spelling it correctly).
So, imho, typo should be pointed out ( with the usual care and sensibility required in every Code Review ) because they could lead to bugs and hard-to-maintain code.
Some linters (we used XO and we love it) help to spot immediately missing functions or properties because:
- you try to access the correct
isMaintenanceproperty - which was not declared (see No-undef)
- `isMantenance´ would be seen as not used (see No-Unused-Vars.
When it comes to Emails and Documents (or Blog Posts) I personally use Grammarly, but when writing code in IntelliJ IDEA I have the Typo Inspections activated so that spelling mistakes are marked with a green squiggly line ( you can define the level of "notification" you prefer and eventually set it as "warning" or "error", so that it stands out even more and no spelling errors slip in)
For Visual Studio I quickly googled and found
I am pretty sure there are plenty more, same for any other IDE you might be using.
These are a couple of spelling mistakes I found recently during our code reviews - and I could not resist pointing them out to the author with a funny link ( sent privately on slack - not in the official MR comment!).
const timestampUnit = "minuets"
The purpose of this **Lambada** function is to ..... (inside a README.md)
Was I too mean?
Have you ever find something funny?
By the way - assuming you are not english speaker, and your team is not multicultural - do you code in english anyway or in your native language?
Photo by Celine Nadon on Unsplash