rucamzu
Joined 9 years ago
Joined 9 years ago
Also, the article doesn't really align with my own understanding and misses a key term: complexity - that's what we abstract.
Abstraction is simply the process of hiding complexity by replacing something complex with something simpler that can better fit in our brains. We abstract complexity all the time, in all aspects of our lives, both consciously and subconsciously. It's not a trait exclusive of software engineering.
First and foremost, I like to think that as a reviewer I'm there not to point out issues but to provide useful feedback to the author. I'm not a compiler, therefore I shouldn't behave like one and dump error messages in the PR. Even if there are errors indeed.
Any and all feedback I provide is up for discussion. This is something I always like to highlight when I'm about to review someone else's code for the first time, no matter their level or experience. I'd rather have my comments discussed and challenged than taken for granted as issues that automatically need to be addressed. I might even throw in the occassional curve ball that I know needs challenging :)
I firmly believe the way I word my comments matters, and matters a lot - and I like to think I've managed to improve a bit over the years. PR comments are not commit messages. I avoid imperatives and quite often phrase comments as questions instead. Also, I favor using "we" as opposed to "you" ; in the end, what gets merged is the result not only of the author's but the whole team's work.
Also, I try to leverage PRs to educate when possible. I frequently include FYI comments illustrating e.g. more idiomatic implementations, potential refactorings or applications of design patterns, not as something I expect the author to fix but to learn from. Code snippets and links to e.g. documentation pages or articles are great ways of enriching a comment and providing guidance beyond the PR itself.
Finally, all these are things that I love to see when someone is reviewing my own PRs.
The two issues the article mentions have imho less to do with the LSP itself, and more with the limitations that different programming languages have when it comes to define contracts through interfaces (not the same thing), like the lack of exception specs or non-nullability enforcement.