Nice post Chris, I agree with most of the things you were describing and the conclusion fits perfectly. Still, I have to disagree with one thing you mentioned which is:
In the end if there are multiple styles in the code base, does it really matter if the maintainers can understand the code? For example, does it really matter that you missed a leading underscore on an instance variable if the rest of the name is intuitive to the team? Or does it really matter if your opening chicken-lips (AKA curly brace or { ) is on the same line as your method declaration or on the line following it? The code still compiles and executes the same regardless...
I mean sure code styling isn't the most important thing in a PR but still most IDEs and dev tools today offer code styling automation, which just eliminates those things from PRs.
So I would conclude: Yes don't be a dick, but also don't make your life unnecessarily hard.
I fully agree with this. In the grand scheme, indentation and brace styles do not matter much and, in the end, we will be able to understand the code (or not) regardless of which style was chosen.
But that's exactly why anyone submitting a PR should not be too lazy to stick to the existing style. Having different styles mixed together is just an unnecessary annoyance and distraction for any reader. Consistency is key.
I agree with Sir Mo. It's not really hard to follow a standard style and all it takes a couple of key strokes. When the code is not formatted properly, it's hard to follow through the code and understand the intent of the developer. At my work, we don't really have PR review meetings as such. We work on agile development and I receive multiple PRs in a day or sometimes a huge PR with 10-20 files modified. If it's not formatted properly, it did took some time to go through and do review on the PR.
BTW it's a great article. I have some challenges on the "Human element" principle, I have been working on it.
Great points guys! Styling is important for all of the reason you guys state. How important it is though, is really something, I have found that needs to come out of the dynamics of the team. For my current team, this means that we don't get upset and block PRs for minor style violations. Instead we may just let the submitter know and merge anyways. We have found for us, this works best. For your guys' teams, it sounds like it is different, and that is also awesome.
I think Sir Mo actually hit the nail on the head as to the point I was trying to make:
don't be a dick, but also don't make your life unnecessarily hard.
There is always a balance to be had, and it is different between teams. In my current team, because we are mostly laid back about style, one person becoming a style "enforcer" (and this has happened) leads to life becoming unecessarily hard. I have worked on other teams where being the lone wolf who does not follow style guidelines and standards would lead to the same result.
Thanks again for the feedback!
For further actions, you may consider blocking this person and/or reporting abuse
We're a place where coders share, stay up-to-date and grow their careers.
Nice post Chris, I agree with most of the things you were describing and the conclusion fits perfectly. Still, I have to disagree with one thing you mentioned which is:
I mean sure code styling isn't the most important thing in a PR but still most IDEs and dev tools today offer code styling automation, which just eliminates those things from PRs.
So I would conclude: Yes don't be a dick, but also don't make your life unnecessarily hard.
I fully agree with this. In the grand scheme, indentation and brace styles do not matter much and, in the end, we will be able to understand the code (or not) regardless of which style was chosen.
But that's exactly why anyone submitting a PR should not be too lazy to stick to the existing style. Having different styles mixed together is just an unnecessary annoyance and distraction for any reader. Consistency is key.
Well said!
I agree with Sir Mo. It's not really hard to follow a standard style and all it takes a couple of key strokes. When the code is not formatted properly, it's hard to follow through the code and understand the intent of the developer. At my work, we don't really have PR review meetings as such. We work on agile development and I receive multiple PRs in a day or sometimes a huge PR with 10-20 files modified. If it's not formatted properly, it did took some time to go through and do review on the PR.
BTW it's a great article. I have some challenges on the "Human element" principle, I have been working on it.
Thanks Chris.
Great points guys! Styling is important for all of the reason you guys state. How important it is though, is really something, I have found that needs to come out of the dynamics of the team. For my current team, this means that we don't get upset and block PRs for minor style violations. Instead we may just let the submitter know and merge anyways. We have found for us, this works best. For your guys' teams, it sounds like it is different, and that is also awesome.
I think Sir Mo actually hit the nail on the head as to the point I was trying to make:
There is always a balance to be had, and it is different between teams. In my current team, because we are mostly laid back about style, one person becoming a style "enforcer" (and this has happened) leads to life becoming unecessarily hard. I have worked on other teams where being the lone wolf who does not follow style guidelines and standards would lead to the same result.
Thanks again for the feedback!