Mass migration to new syntax - PRO or CON?

classic Classic list List threaded Threaded
30 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Mass migration to new syntax - PRO or CON?

Daniel Kinzler
Now that we target PHP 5.5, some people are itching to make use of some new
language features, like the new array syntax, e.g.
<https://gerrit.wikimedia.org/r/#/c/269745/>.

Mass changes like this, or similar changes relating to coding style, tend to
lead to controversy. I want to make sure we have a discussion about this here,
to avoid having the argument over and over on any such patch.

Please give a quick PRO or CON response as a basis for discussion.

In essence, the discussion boils down to two conflicting positions:

PRO: do mass migration to the new syntax, style, or whatever, as soon as
possible. This way, the codebase is in a consistent form, and that form is the
one we agreed is the best for readability. Doing changes like this is
gratifying, because it's low hanging fruit: it's easy to do, and has large
visible impact (well ok, visible in the source).

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.

-- daniel

_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Mass migration to new syntax - PRO or CON?

Chad
On Fri, Feb 12, 2016 at 7:27 AM Daniel Kinzler <[hidden email]> 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
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Mass migration to new syntax - PRO or CON?

Marius Hoch
In reply to this post by Daniel Kinzler
I also think we shouldn't mass migrate.

New code should use the new syntax and old code can be converted during
larger refactors or similar things (when it is being touched anyway),
but we shouldn't have "update syntax" only patches.

Cheers,

Marius

On 12.02.2016 16:27, Daniel Kinzler wrote:

> Now that we target PHP 5.5, some people are itching to make use of some new
> language features, like the new array syntax, e.g.
> <https://gerrit.wikimedia.org/r/#/c/269745/>.
>
> Mass changes like this, or similar changes relating to coding style, tend to
> lead to controversy. I want to make sure we have a discussion about this here,
> to avoid having the argument over and over on any such patch.
>
> Please give a quick PRO or CON response as a basis for discussion.
>
> In essence, the discussion boils down to two conflicting positions:
>
> PRO: do mass migration to the new syntax, style, or whatever, as soon as
> possible. This way, the codebase is in a consistent form, and that form is the
> one we agreed is the best for readability. Doing changes like this is
> gratifying, because it's low hanging fruit: it's easy to do, and has large
> visible impact (well ok, visible in the source).
>
> 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.
>
> -- daniel
>
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Mass migration to new syntax - PRO or CON?

Jon Robson-2
In reply to this post by Chad
On Fri, Feb 12, 2016 at 9:14 AM, Chad <[hidden email]> wrote:

> On Fri, Feb 12, 2016 at 7:27 AM Daniel Kinzler <[hidden email]>
> 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*
>

Agree on many levels.
Please, let's focus on solving problems and improving how our code works
rather than how our code looks.

>
> -Chad
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Mass migration to new syntax - PRO or CON?

Purodha Blissenbach
CON, for all the reasons mentioned.

Purodha


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Mass migration to new syntax - PRO or CON?

Stas Malyshev
In reply to this post by Daniel Kinzler
Hi!

> Please give a quick PRO or CON response as a basis for discussion.

My opinion: if it ain't broke, don't fix it. It's ok to use new syntax
in new code, but spending time on changing perfectly working code just
to use new array syntax looks like misplaced effort to me.

There are new features that I would have more support immediate change,
like $this in closures - that makes code much more readable and less
bug-prone. Even then, I'm ambivalent whether we need to touch the code
that much, but I see a point of cleaning it up. But with array syntax,
it's just a different syntax, and I don't see a lot of reason to mess
with existing working code just to use it.
--
Stas Malyshev
[hidden email]

_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Mass migration to new syntax - PRO or CON?

Toby Negrin
In reply to this post by Purodha Blissenbach
First, a thank you for Daniel for writing such a balanced, informative
email on a somewhat contentious subject!

As someone who will probably never touch a line of MW code, I'm not going
to actually vote. But I do have experience in large, complex codebases and
I'm going to argue for PRO.

We need to acknowledge that our codebase is going to change over time and
if we aren't explicit and accepting of this reality, we're not going to be
able to manage it well. Consider future developers who may only ever know
new syntax and their desire and ability to work on a codebase that doesn't
reflect new techniques and syntax in its implementation.

-Toby

On Fri, Feb 12, 2016 at 9:44 AM, Purodha Blissenbach <
[hidden email]> wrote:

> CON, for all the reasons mentioned.
>
> Purodha
>
>
>
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Mass migration to new syntax - PRO or CON?

Ori Livneh
In reply to this post by Daniel Kinzler
On Fri, Feb 12, 2016 at 7:27 AM, Daniel Kinzler <[hidden email]>
wrote:

> Now that we target PHP 5.5, some people are itching to make use of some new
> language features, like the new array syntax, e.g.
> <https://gerrit.wikimedia.org/r/#/c/269745/>.
>
> Mass changes like this, or similar changes relating to coding style, tend
> to
> lead to controversy. I want to make sure we have a discussion about this
> here,
> to avoid having the argument over and over on any such patch.
>
> Please give a quick PRO or CON response as a basis for discussion.
>

PRO. These syntax changes were implemented in PHP at the cost of breaking
backward-compatibility, which tells you that people understood their value
and were willing to pay a cost for modernizing and simplifying the
language. If PHP was willing to pay it, why wouldn't we?
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Mass migration to new syntax - PRO or CON?

Stas Malyshev
Hi!

> PRO. These syntax changes were implemented in PHP at the cost of breaking
> backward-compatibility, which tells you that people understood their value

Wait, are we talking about the same thing? New array syntax does not
break BC. Or you mean that if we use new array syntax, we'd break BC
with older PHP versions? I'm not sure I understand your argument here.

--
Stas Malyshev
[hidden email]

_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Mass migration to new syntax - PRO or CON?

Jon Robson-2
On Fri, Feb 12, 2016 at 10:26 AM, Stas Malyshev <[hidden email]>
wrote:

> Hi!
>
> > PRO. These syntax changes were implemented in PHP at the cost of breaking
> > backward-compatibility, which tells you that people understood their
> value
>
> Wait, are we talking about the same thing? New array syntax does not
> break BC. Or you mean that if we use new array syntax, we'd break BC
> with older PHP versions? I'm not sure I understand your argument here.
>

I'm also a little puzzled. I thought it was a given we are embracing and
modernizing newer PHP versions.

The question as I understood it, is should we touch every piece of our
codebase in one big mega patch or update it gradually as and when we visit
bits of the codebase (I get the impression the latter isn't happening due
to a desire to have mega patches).


>
> --
> Stas Malyshev
> [hidden email]
>
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Mass migration to new syntax - PRO or CON?

Stas Malyshev
Hi!

> The question as I understood it, is should we touch every piece of our
> codebase in one big mega patch or update it gradually as and when we visit
> bits of the codebase (I get the impression the latter isn't happening due
> to a desire to have mega patches).

Same here and just to be clear, I'm for the gradual approach.

--
Stas Malyshev
[hidden email]

_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Mass migration to new syntax - PRO or CON?

Legoktm
In reply to this post by Daniel Kinzler
Hi,

On 02/12/2016 07:27 AM, Daniel Kinzler wrote:

> Now that we target PHP 5.5, some people are itching to make use of some new
> language features, like the new array syntax, e.g.
> <https://gerrit.wikimedia.org/r/#/c/269745/>.
>
> Mass changes like this, or similar changes relating to coding style, tend to
> lead to controversy. I want to make sure we have a discussion about this here,
> to avoid having the argument over and over on any such patch.
>
> Please give a quick PRO or CON response as a basis for discussion.
>
> In essence, the discussion boils down to two conflicting positions:
>
> PRO: do mass migration to the new syntax, style, or whatever, as soon as
> possible. This way, the codebase is in a consistent form, and that form is the
> one we agreed is the best for readability. Doing changes like this is
> gratifying, because it's low hanging fruit: it's easy to do, and has large
> visible impact (well ok, visible in the source).

I'll offer an alternative, which is to convert all of them at once using
PHPCS and then enforce that all new patches use [] arrays. You then only
have one commit which changes everything, not hundreds you have to go
through while git blaming or looking in git log.

> 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?

There's no need to do it manually. Just tell people to run the phpcs
autofixer before they rebase, and the result should be identical to
what's already there. And we can have PHPCS run in the other direction
for backports ([] -> array()).

But if we don't do that, people are going to start converting things
manually whenever they work on the code, and you'll still end up with
hundreds of open patches needing rebase, except it can't be done
automatically anymore.

> My personal vote is CON. No rebase hell please! Changing to the syntax doesn't
> buy us anything.

Consistency buys us a lot. New developers won't be confused on whether
to use [] or array(). It makes entry easier for people coming from other
languages where [] is used for lists.

I think you're going to end up in rebase hell regardless, so we should
rip off the bandaid quickly and get it over with, and use the automated
tools we have to our advantage.

So, if we're voting, I'm PRO.

-- Legoktm

_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Mass migration to new syntax - PRO or CON?

Alex Monk
PRO from me, for all the reasons mentioned by legoktm

On 12 February 2016 at 19:26, Legoktm <[hidden email]> wrote:

> Hi,
>
> On 02/12/2016 07:27 AM, Daniel Kinzler wrote:
> > Now that we target PHP 5.5, some people are itching to make use of some
> new
> > language features, like the new array syntax, e.g.
> > <https://gerrit.wikimedia.org/r/#/c/269745/>.
> >
> > Mass changes like this, or similar changes relating to coding style,
> tend to
> > lead to controversy. I want to make sure we have a discussion about this
> here,
> > to avoid having the argument over and over on any such patch.
> >
> > Please give a quick PRO or CON response as a basis for discussion.
> >
> > In essence, the discussion boils down to two conflicting positions:
> >
> > PRO: do mass migration to the new syntax, style, or whatever, as soon as
> > possible. This way, the codebase is in a consistent form, and that form
> is the
> > one we agreed is the best for readability. Doing changes like this is
> > gratifying, because it's low hanging fruit: it's easy to do, and has
> large
> > visible impact (well ok, visible in the source).
>
> I'll offer an alternative, which is to convert all of them at once using
> PHPCS and then enforce that all new patches use [] arrays. You then only
> have one commit which changes everything, not hundreds you have to go
> through while git blaming or looking in git log.
>
> > 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?
>
> There's no need to do it manually. Just tell people to run the phpcs
> autofixer before they rebase, and the result should be identical to
> what's already there. And we can have PHPCS run in the other direction
> for backports ([] -> array()).
>
> But if we don't do that, people are going to start converting things
> manually whenever they work on the code, and you'll still end up with
> hundreds of open patches needing rebase, except it can't be done
> automatically anymore.
>
> > My personal vote is CON. No rebase hell please! Changing to the syntax
> doesn't
> > buy us anything.
>
> Consistency buys us a lot. New developers won't be confused on whether
> to use [] or array(). It makes entry easier for people coming from other
> languages where [] is used for lists.
>
> I think you're going to end up in rebase hell regardless, so we should
> rip off the bandaid quickly and get it over with, and use the automated
> tools we have to our advantage.
>
> So, if we're voting, I'm PRO.
>
> -- Legoktm
>
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Mass migration to new syntax - PRO or CON?

Joaquin Oltra Hernandez
PRO from me too.

Doing it gradually is just going to make the codebase inconsistent, and
tooling can help point patches to the old style to migrate to the new one.

I'd rather do it quickly than have the inconsistency bleed through months
or years.

On Fri, Feb 12, 2016 at 8:28 PM, Alex Monk <[hidden email]> wrote:

> PRO from me, for all the reasons mentioned by legoktm
>
> On 12 February 2016 at 19:26, Legoktm <[hidden email]> wrote:
>
> > Hi,
> >
> > On 02/12/2016 07:27 AM, Daniel Kinzler wrote:
> > > Now that we target PHP 5.5, some people are itching to make use of some
> > new
> > > language features, like the new array syntax, e.g.
> > > <https://gerrit.wikimedia.org/r/#/c/269745/>.
> > >
> > > Mass changes like this, or similar changes relating to coding style,
> > tend to
> > > lead to controversy. I want to make sure we have a discussion about
> this
> > here,
> > > to avoid having the argument over and over on any such patch.
> > >
> > > Please give a quick PRO or CON response as a basis for discussion.
> > >
> > > In essence, the discussion boils down to two conflicting positions:
> > >
> > > PRO: do mass migration to the new syntax, style, or whatever, as soon
> as
> > > possible. This way, the codebase is in a consistent form, and that form
> > is the
> > > one we agreed is the best for readability. Doing changes like this is
> > > gratifying, because it's low hanging fruit: it's easy to do, and has
> > large
> > > visible impact (well ok, visible in the source).
> >
> > I'll offer an alternative, which is to convert all of them at once using
> > PHPCS and then enforce that all new patches use [] arrays. You then only
> > have one commit which changes everything, not hundreds you have to go
> > through while git blaming or looking in git log.
> >
> > > 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?
> >
> > There's no need to do it manually. Just tell people to run the phpcs
> > autofixer before they rebase, and the result should be identical to
> > what's already there. And we can have PHPCS run in the other direction
> > for backports ([] -> array()).
> >
> > But if we don't do that, people are going to start converting things
> > manually whenever they work on the code, and you'll still end up with
> > hundreds of open patches needing rebase, except it can't be done
> > automatically anymore.
> >
> > > My personal vote is CON. No rebase hell please! Changing to the syntax
> > doesn't
> > > buy us anything.
> >
> > Consistency buys us a lot. New developers won't be confused on whether
> > to use [] or array(). It makes entry easier for people coming from other
> > languages where [] is used for lists.
> >
> > I think you're going to end up in rebase hell regardless, so we should
> > rip off the bandaid quickly and get it over with, and use the automated
> > tools we have to our advantage.
> >
> > So, if we're voting, I'm PRO.
> >
> > -- Legoktm
> >
> > _______________________________________________
> > Wikitech-l mailing list
> > [hidden email]
> > https://lists.wikimedia.org/mailman/listinfo/wikitech-l
> >
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Mass migration to new syntax - PRO or CON?

Tyler Romeo
I think it's also pertinent to note that we are finally reaching a state where PHP-CS can be voting, which was only achieved after a lot of hard work to make our codebase consistent.

If we make these changes gradually, we're basically throwing away all of the work that was just done recently.

Regards,
-- 
Tyler Romeo
https://parent5446.nyc
0x405D34A7C86B42DF

From: Joaquin Oltra Hernandez <[hidden email]>
Reply: Wikimedia developers <[hidden email]>
Date: February 12, 2016 at 14:31:54
To: Wikimedia developers <[hidden email]>
Subject:  Re: [Wikitech-l] Mass migration to new syntax - PRO or CON?  

PRO from me too.

Doing it gradually is just going to make the codebase inconsistent, and
tooling can help point patches to the old style to migrate to the new one.

I'd rather do it quickly than have the inconsistency bleed through months
or years.

On Fri, Feb 12, 2016 at 8:28 PM, Alex Monk <[hidden email]> wrote:

> PRO from me, for all the reasons mentioned by legoktm
>
> On 12 February 2016 at 19:26, Legoktm <[hidden email]> wrote:
>
> > Hi,
> >
> > On 02/12/2016 07:27 AM, Daniel Kinzler wrote:
> > > Now that we target PHP 5.5, some people are itching to make use of some
> > new
> > > language features, like the new array syntax, e.g.
> > > <https://gerrit.wikimedia.org/r/#/c/269745/>.
> > >
> > > Mass changes like this, or similar changes relating to coding style,
> > tend to
> > > lead to controversy. I want to make sure we have a discussion about
> this
> > here,
> > > to avoid having the argument over and over on any such patch.
> > >
> > > Please give a quick PRO or CON response as a basis for discussion.
> > >
> > > In essence, the discussion boils down to two conflicting positions:
> > >
> > > PRO: do mass migration to the new syntax, style, or whatever, as soon
> as
> > > possible. This way, the codebase is in a consistent form, and that form
> > is the
> > > one we agreed is the best for readability. Doing changes like this is
> > > gratifying, because it's low hanging fruit: it's easy to do, and has
> > large
> > > visible impact (well ok, visible in the source).
> >
> > I'll offer an alternative, which is to convert all of them at once using
> > PHPCS and then enforce that all new patches use [] arrays. You then only
> > have one commit which changes everything, not hundreds you have to go
> > through while git blaming or looking in git log.
> >
> > > 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?
> >
> > There's no need to do it manually. Just tell people to run the phpcs
> > autofixer before they rebase, and the result should be identical to
> > what's already there. And we can have PHPCS run in the other direction
> > for backports ([] -> array()).
> >
> > But if we don't do that, people are going to start converting things
> > manually whenever they work on the code, and you'll still end up with
> > hundreds of open patches needing rebase, except it can't be done
> > automatically anymore.
> >
> > > My personal vote is CON. No rebase hell please! Changing to the syntax
> > doesn't
> > > buy us anything.
> >
> > Consistency buys us a lot. New developers won't be confused on whether
> > to use [] or array(). It makes entry easier for people coming from other
> > languages where [] is used for lists.
> >
> > I think you're going to end up in rebase hell regardless, so we should
> > rip off the bandaid quickly and get it over with, and use the automated
> > tools we have to our advantage.
> >
> > So, if we're voting, I'm PRO.
> >
> > -- Legoktm
> >
> > _______________________________________________
> > Wikitech-l mailing list
> > [hidden email]
> > https://lists.wikimedia.org/mailman/listinfo/wikitech-l
> >
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

signature.asc (852 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Mass migration to new syntax - PRO or CON?

Max Semenik
In reply to this post by Alex Monk
PRO: preserving the blame history is a false hope because we liberally move
files around, extract classes to separate files and so on. In other words,
we do the usual refactoring work and in process we break blame at all
times. Syntax updates are just another refactoring, nothing principally
new. And no, when hell freezes over are inconsistent code standards better:
we already have a code base inconsistent enough after 15 years of
development and MUST avoid creating more.

On Fri, Feb 12, 2016 at 11:28 AM, Alex Monk <[hidden email]> wrote:

> PRO from me, for all the reasons mentioned by legoktm
>
> On 12 February 2016 at 19:26, Legoktm <[hidden email]> wrote:
>
> > Hi,
> >
> > On 02/12/2016 07:27 AM, Daniel Kinzler wrote:
> > > Now that we target PHP 5.5, some people are itching to make use of some
> > new
> > > language features, like the new array syntax, e.g.
> > > <https://gerrit.wikimedia.org/r/#/c/269745/>.
> > >
> > > Mass changes like this, or similar changes relating to coding style,
> > tend to
> > > lead to controversy. I want to make sure we have a discussion about
> this
> > here,
> > > to avoid having the argument over and over on any such patch.
> > >
> > > Please give a quick PRO or CON response as a basis for discussion.
> > >
> > > In essence, the discussion boils down to two conflicting positions:
> > >
> > > PRO: do mass migration to the new syntax, style, or whatever, as soon
> as
> > > possible. This way, the codebase is in a consistent form, and that form
> > is the
> > > one we agreed is the best for readability. Doing changes like this is
> > > gratifying, because it's low hanging fruit: it's easy to do, and has
> > large
> > > visible impact (well ok, visible in the source).
> >
> > I'll offer an alternative, which is to convert all of them at once using
> > PHPCS and then enforce that all new patches use [] arrays. You then only
> > have one commit which changes everything, not hundreds you have to go
> > through while git blaming or looking in git log.
> >
> > > 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?
> >
> > There's no need to do it manually. Just tell people to run the phpcs
> > autofixer before they rebase, and the result should be identical to
> > what's already there. And we can have PHPCS run in the other direction
> > for backports ([] -> array()).
> >
> > But if we don't do that, people are going to start converting things
> > manually whenever they work on the code, and you'll still end up with
> > hundreds of open patches needing rebase, except it can't be done
> > automatically anymore.
> >
> > > My personal vote is CON. No rebase hell please! Changing to the syntax
> > doesn't
> > > buy us anything.
> >
> > Consistency buys us a lot. New developers won't be confused on whether
> > to use [] or array(). It makes entry easier for people coming from other
> > languages where [] is used for lists.
> >
> > I think you're going to end up in rebase hell regardless, so we should
> > rip off the bandaid quickly and get it over with, and use the automated
> > tools we have to our advantage.
> >
> > So, if we're voting, I'm PRO.
> >
> > -- Legoktm
> >
> > _______________________________________________
> > Wikitech-l mailing list
> > [hidden email]
> > https://lists.wikimedia.org/mailman/listinfo/wikitech-l
> >
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>



--
Best regards,
Max Semenik ([[User:MaxSem]])
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Mass migration to new syntax - PRO or CON?

Brad Jorsch (Anomie)
In reply to this post by Legoktm
On Fri, Feb 12, 2016 at 2:26 PM, Legoktm <[hidden email]>
wrote:

> I think you're going to end up in rebase hell regardless, so we should
> rip off the bandaid quickly and get it over with, and use the automated
> tools we have to our advantage.
>

This. Just get it over with, and then it's only one patch screwing up
rebases and blames instead of lots.



--
Brad Jorsch (Anomie)
Senior Software Engineer
Wikimedia Foundation
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Mass migration to new syntax - PRO or CON?

Ricordisamoa
In reply to this post by Legoktm
Il 12/02/2016 20:26, Legoktm ha scritto:

> Hi,
>
> On 02/12/2016 07:27 AM, Daniel Kinzler wrote:
>> Now that we target PHP 5.5, some people are itching to make use of some new
>> language features, like the new array syntax, e.g.
>> <https://gerrit.wikimedia.org/r/#/c/269745/>.
>>
>> Mass changes like this, or similar changes relating to coding style, tend to
>> lead to controversy. I want to make sure we have a discussion about this here,
>> to avoid having the argument over and over on any such patch.
>>
>> Please give a quick PRO or CON response as a basis for discussion.
>>
>> In essence, the discussion boils down to two conflicting positions:
>>
>> PRO: do mass migration to the new syntax, style, or whatever, as soon as
>> possible. This way, the codebase is in a consistent form, and that form is the
>> one we agreed is the best for readability. Doing changes like this is
>> gratifying, because it's low hanging fruit: it's easy to do, and has large
>> visible impact (well ok, visible in the source).
> I'll offer an alternative, which is to convert all of them at once using
> PHPCS and then enforce that all new patches use [] arrays. You then only
> have one commit which changes everything, not hundreds you have to go
> through while git blaming or looking in git log.
>
>> 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?
> There's no need to do it manually. Just tell people to run the phpcs
> autofixer before they rebase, and the result should be identical to
> what's already there. And we can have PHPCS run in the other direction
> for backports ([] -> array()).
>
> But if we don't do that, people are going to start converting things
> manually whenever they work on the code, and you'll still end up with
> hundreds of open patches needing rebase, except it can't be done
> automatically anymore.
>
>> My personal vote is CON. No rebase hell please! Changing to the syntax doesn't
>> buy us anything.
> Consistency buys us a lot. New developers won't be confused on whether
> to use [] or array(). It makes entry easier for people coming from other
> languages where [] is used for lists.

Objection: other languages may use [] for lists, but array() is more
similar to {} used for hash tables.

>
> I think you're going to end up in rebase hell regardless, so we should
> rip off the bandaid quickly and get it over with, and use the automated
> tools we have to our advantage.
>
> So, if we're voting, I'm PRO.
>
> -- Legoktm
>
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Mass migration to new syntax - PRO or CON?

Denny Vrandečić-2
In reply to this post by Brad Jorsch (Anomie)
Although I haven't touched MediaWiki code for a year or so, based on my
experience with large codebases with tons of contributors, I would be very
much PRO.

I understand it is a pain, but as Legoktm points out, it is a manageable
pain. Having a consistent and higher-quality code base is worth the
migration pain. Three more advantages:
* future changesets are cleaner, as one does not have to do the clean up in
addition to the actual change they wanted to do
* automatic testing tools can capture issues with a higher confidence if it
doesn't have to take historical exceptions into account
* most developers code by copy-and-paste of style, structures, and ideas.
So even if a new styleguide is in place, it can often be the case that a
developer will start building off the old styleguide as they simply keep
their code consistent with the code that they are looking at

hth


On Fri, Feb 12, 2016 at 11:57 AM Brad Jorsch (Anomie) <[hidden email]>
wrote:

> On Fri, Feb 12, 2016 at 2:26 PM, Legoktm <[hidden email]>
> wrote:
>
> > I think you're going to end up in rebase hell regardless, so we should
> > rip off the bandaid quickly and get it over with, and use the automated
> > tools we have to our advantage.
> >
>
> This. Just get it over with, and then it's only one patch screwing up
> rebases and blames instead of lots.
>
>
>
> --
> Brad Jorsch (Anomie)
> Senior Software Engineer
> Wikimedia Foundation
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Mass migration to new syntax - PRO or CON?

Chad
In reply to this post by Chad
On Fri, Feb 12, 2016 at 9:14 AM Chad <[hidden email]> wrote:

> On Fri, Feb 12, 2016 at 7:27 AM Daniel Kinzler <[hidden email]>
> 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
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
12