I'm not going to touch much PHP code any time soon, so feel free to ignore my opinion.
I'm all for a consistent code base, or at least for a clear rule, even if a bit arbitrary. Style issues are too often a matter of taste, so having a clear and enforced rule allows us to stop the discussion and focus on something more important.
Since we should have a rule, that rule should be as much as possible the "state of the art". In this case it seems clear that new array syntax is a winner.
Now that we have a rule, we need to enforce it (style check during CI). This is a clear communication of the intent and again helps us stop wasting time on manual reviews to enforce the rule. Code review time is precious and should be spent on important stuff.
Last question: how do we manage the transition? Changing perfectly working code just for the sake of style seems a bit like wasted resources. So we should ensure that new code, or any code that is touched follows the new rule, we ignore existing untouched code. We need the appropriate tooling to make that possible, and that's why I need to find some time to deploy a SonarQube instance as a proof of concept.
Sorry if I'm rambling, it's a bit late and my English is rusty... but I always find discussions about code style interesting. Mostly because I want us to stop having them ...
On Fri, Feb 12, 2016 at 9:54 PM, Gabriel Wicke gwicke@wikimedia.org wrote:
Overall I'm PRO, as consistency is worth a lot, and tools can apply such changes consistently and efficiently.
We have applied broad formatting changes to large JS codebases using jscs, which has worked well when those changes were well prepared. Typically, this involved gradually refining the tool settings until a reasonable diff was achieved.
On Fri, Feb 12, 2016 at 12:44 PM, Chad innocentkiller@gmail.com wrote:
On Fri, Feb 12, 2016 at 9:14 AM Chad innocentkiller@gmail.com wrote:
On Fri, Feb 12, 2016 at 7:27 AM Daniel Kinzler daniel@brightbyte.de wrote:
CON: don't do mass migration to new syntax, only start using new styles and features when touching the respective bit of code anyway. The argument
is
here that touching many lines of code, even if it's just for whitespace changes, causes merge conflicts when doing backports and when rebasing patches. E.g. if we touch half the files in the codebase to change to the new array syntax, who is going to manually rebase the couple of hundred patches we have open?
As can be seen on the proposed patch I linked, several of the long term developers oppose mass changes like this. A quick round of feedback in
the
architecture committee draws a similar picture. However, perhaps there
are
compelling arguments for doing the mass migration that we haven't heard yet. So please give a quick PRO or CON, optionally with some rationale.
My personal vote is CON. No rebase hell please! Changing to the syntax doesn't buy us anything.
CON, for all the reasons you mentioned. Also: style only changes are
pain
when you're trying to annotate/blame a particular line of code.
ESPECIALLY for something so silly as array formatting which gains us *absolutely nothing*
-Chad
I change my vote to PRO.
Mainly because people are gonna do it anyway...
Last thoughts on the thread, I got bigger fish to fry than array syntax sugar :D
-Chad _______________________________________________ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
-- Gabriel Wicke Principal Engineer, Wikimedia Foundation
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l