Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [eclipse-pmc] Review of mass changes

> So, anyone against setting M1 as the deadline?

I suggest to discuss this in our next PMC call. Alex, is out this week
and I'm sure he has an opinion on that ?

Also, I think it is easier to discuss things like "what is a mass
change?", "what if it is really trivial?", etc directly.

Best regards, Lars

On Fri, Jun 21, 2019 at 4:13 PM Daniel Megert <daniel_megert@xxxxxxxxxx> wrote:
>
> > and not every milestone as decided.
>
> As I mentioned before, the deadline milestone for mass changes has not yet been decided. Currently it looks like M1 it will be,
>
> So, anyone against setting M1 as the deadline?
>
> Dani
>
>
>
> From:        "Manoj Palat" <manoj.palat@xxxxxxxxxx>
> To:        eclipse-pmc@xxxxxxxxxxx
> Date:        21.06.2019 12:20
> Subject:        [EXTERNAL] Re: [eclipse-pmc] Review of mass changes
> Sent by:        eclipse-pmc-bounces@xxxxxxxxxxx
> ________________________________
>
>
>
> Hi Lars,
>
> Just a clarification in case there is a confusion - 2-3 mass changes per every M1 milestone and not every milestone as decided.
>
> Thanks,
> Manoj.
>
>
> Lars Vogel ---06/21/2019 03:20:03 PM---Hi Manoj, > This also brings me to the question - as how many mass changes should we consider reason
>
> From: Lars Vogel <lars.vogel@xxxxxxxxxxx>
> To: eclipse-pmc@xxxxxxxxxxx
> Date: 06/21/2019 03:20 PM
> Subject: [EXTERNAL] Re: [eclipse-pmc] Review of mass changes
> Sent by: eclipse-pmc-bounces@xxxxxxxxxxx
> ________________________________
>
>
>
> Hi Manoj,
>
> > This also brings me to the question - as how many mass changes should we consider reasonable to accept?
>
> IMHO this depends on the project state. If for example JDT core would
> accept 2-3 mass changes per milestone I think that would be very
> beneficial.
>
> In Platform we recently saw a huge amount of cleanup patches from
> Carsten Hammer (thanks to Carsten for working on this) and thanks to
> our very community orientated committers we integrated most of them
> within a week or two after master was open again.
>
> Therefore, I don't think we should put a max limit of this work.
>
> I also assume that within platform code the opportunity for mass
> changes is getting smaller, as the code base has already received a
> lot of improvements (or "unless changes" depending on the point of
> view ;-)).
>
> I created two tiny mass changes ;-) for JDT core to test the suggested
> process, please have a look and let me know what you think and if you
> require cherry-picks for them. I suggest we do this discussion in the
> Gerrits.
>
> Best regards, Lars
>
>
>
>
> On Fri, Jun 21, 2019 at 11:35 AM Manoj Palat <manoj.palat@xxxxxxxxxx> wrote:
> >
> > Hi All,
> > This also brings me to the question - as how many mass changes should we consider reasonable to accept? I think 2 or 3 mass changes per milestone per component looks reasonable - both from the amount of code change and the committer's efforts. Let us come to a conclusion on this figure as well.
> >
> > Thanks Lars for bringing up the points. I have answered inline.
> >
> > eclipse-pmc-bounces@xxxxxxxxxxx wrote on 06/21/2019 12:03:06 PM:
> > > From: Lars Vogel <lars.vogel@xxxxxxxxxxx>
> >
> >
> > > A small comment to the suggestion for JDT merges
> > >
> > > > @Dani:
> > > > I would recommend the following strategy for JDT:
> > > > 1) author merging the massive changes first into master
> > > I think you mean committer here. Author may be a contributor
> >
> > 1) You are right, If author is a contributor, let him/her just submit the gerrit and then ask for commit after the tests are green [ and after he has a gerrit patch for BETA_JAVAn - see point 2].
> >
> > > > 2) then the author himself creating a gerrit (after conflict
> > > resolutions, if any) for the BETA_JAVAn branch.
> > > IMHO this might be the committer or the author. Reviewing a cherry-
> > > picked commit might be more work for the committer than doing the
> > > merge/cherry-pick directly.
> >
> > 2) I would still vote for the author - if not conflict resolutions would take away a major bandwidth of the author. And I would request all who submit to JDT to create the patch for the BETA_JAVAn branch as well simultaneously - to make sure there are not merge conflicts. we can cherry-pick the one from master if all's fine.
> >
> > Further, at least from JDT point of view:
> >
> > a) Let every mass change have a bug - its easy to track.
> >
> >
> >
> > > > 3) One of the committers working on the BETA_JAVAn branch can
> > > commit in the next merge from master to BETA_JAVAn
> > >
> > > +1
> > >
> > > Manoj, later today I plan to create a simple patch for JDT.core and
> > > add you as reviewer so that we can test the suggested process. I
> > > hope that is OK for you.
> >
> > @Lars: Sure - will take a look. You have already using up one out of the total mass changes allowed :)
> >
> >
> > > Best regards, Lars
> > >
> > > On Fri, Jun 21, 2019 at 6:36 AM Manoj Palat <manoj.palat@xxxxxxxxxx> wrote:
> > > @Lars : Please find the outline of merge strategy of JDT
> > >
> > > (1) JDT creates a BETA_JAVAn [where n=13 currently] for every Java
> > > release and the merge from the master to the BETA branch is done
> > > regularly - in terms of (1) periodically every milestone (2)
> > > regularly if there are major changes in master and (3) if there are
> > > major changes in BETA_JAVAn itself.
> > >
> > > (2) It should also be noted that the BETA_JAVAn will merge to master
> > > every six months - and this merge is fixed to be done in M1 - in
> > > fact early M1 - mostly as one of the initial commits given the close
> > > release dates of both Eclipse - immediately after March and
> > > September releases.
> > >
> > > (3) BETA_JAVAn will continue to be available as a Patch on March and
> > > September releases of Eclipse - Hence again it is required that the
> > > BETA_JAVAn is in sync with master for a smooth patch update.
> > >
> > > @Dani:
> > > I would recommend the following strategy for JDT:
> > > 1) author merging the massive changes first into master
> > > 2) then the author himself creating a gerrit (after conflict
> > > resolutions, if any) for the BETA_JAVAn branch.
> > > 3) One of the committers working on the BETA_JAVAn branch can commit
> > > in the next merge from master to BETA_JAVAn
> > >
> > > We can start with this strategy and see how it goes, and then change
> > > accordingly.
> > >
> > > Regards,
> > > Manoj.
> > >
> > >
> > > [image removed] Lars Vogel ---06/20/2019 09:12:13 PM---What is the
> > > merge strategy in JDT? Do they merge master into the feature branch
> > > on a regular basis?
> > >
> > > From: Lars Vogel <lars.vogel@xxxxxxxxxxx>
> > > To: eclipse-pmc@xxxxxxxxxxx
> > > Date: 06/20/2019 09:12 PM
> > > Subject: [EXTERNAL] Re: [eclipse-pmc] Review of mass changes
> > > Sent by: eclipse-pmc-bounces@xxxxxxxxxxx
> > >
> > >
> > >
> > > What is the merge strategy in JDT? Do they merge master into the
> > > feature branch on a regular basis?
> > >
> > > Daniel Megert <daniel_megert@xxxxxxxxxx> schrieb am Do., 20. Juni
> > > 2019, 17:21:
> > > The milestone has not yet been decided. Mickael, I would be fine with M1.
> > >
> > > What would be better for JDT: Also getting the same change for the
> > > branch or do the merge yourself along with all other changes?
> > >
> > > Dani
> > >
> > >
> > >
> > > From:        "Manoj Palat" <manoj.palat@xxxxxxxxxx>
> > > To:        eclipse-pmc@xxxxxxxxxxx
> > > Date:        20.06.2019 06:55
> > > Subject:        [EXTERNAL] Re: [eclipse-pmc] Review of mass changes
> > > Sent by:        eclipse-pmc-bounces@xxxxxxxxxxx
> > >
> > >
> > >
> > > Hi All,
> > >
> > > Thanks Sarika for bringing this point out.
> > >
> > > A mass change which has proven benefits is always welcome and really
> > > appreciate the time and effort of people who do that.
> > >
> > > However, JDT changes themselve are massive and frequent due to the
> > > interplay of three month release of Eclipse combined with the six
> > > month release cadence of Java - sometimes the merging efforts
> > > themselves vie for being a subproject of Eclipse :).
> > >
> > > Given these two factors, I totally agree especially from JDT
> > > projects pov, M1 can be (and should be the only) the point in which
> > > such proven mass changes can be incorporated.
> > >
> > > Regards,
> > > Manoj
> > >
> > > "Sarika Sinha" ---06/20/2019 09:58:49 AM---There are few points to
> > > be considered - 1. Problem is not with the trivial code changes, problem is
> > >
> > > From: "Sarika Sinha" <sarika.sinha@xxxxxxxxxx>
> > > To: eclipse-pmc@xxxxxxxxxxx
> > > Date: 06/20/2019 09:58 AM
> > > Subject: [EXTERNAL] Re: [eclipse-pmc] Review of mass changes
> > > Sent by: eclipse-pmc-bounces@xxxxxxxxxxx
> > >
> > >
> > >
> > > There are few points to be considered -
> > > 1. Problem is not with the trivial code changes, problem is in
> > > managing the merge conflicts either during this mass change by the
> > > contributor or by other contributors after this mass change is
> > > released. (As I have seen it happening this week where contributor
> > > is struggling to rebase the mass change gerrits)
> > > 2. This merging turns to be an evil specially for JDT repositories
> > > where new Java version work happens in a different branch (due to
> > > legal constraints).
> > >
> > > I am OK with the mass changes in M1 but it should be a call of the
> > > component based on the timeline and other feature development going
> > > on in parallel like JDT.
> > >
> > > Thanks & Regards,
> > > Sarika
> > >
> > > "Daniel Megert" ---06/20/2019 06:01:32 AM---Thanks Mickael, that's a
> > > good approach which works fine with me. As for the review, yes a second per
> > >
> > > From: "Daniel Megert" <daniel_megert@xxxxxxxxxx>
> > > To: eclipse-pmc@xxxxxxxxxxx
> > > Date: 06/20/2019 06:01 AM
> > > Subject: [EXTERNAL] Re: [eclipse-pmc] Review of mass changes
> > > Sent by: eclipse-pmc-bounces@xxxxxxxxxxx
> > >
> > >
> > >
> > > Thanks Mickael, that's a good approach which works fine with me.
> > >
> > > As for the review, yes a second person besides the owner must give a
> > > code-review+1 (or+2), but for mass changes with (apparently) trivial
> > > code changes I would be OK if only random samples are reviewed.
> > >
> > > Dani
> > >
> > >
> > >
> > > From: Mickael Istria <mistria@xxxxxxxxxx>
> > > To: "eclipse-pmc@xxxxxxxxxxx" <eclipse-pmc@xxxxxxxxxxx>
> > > Date: 19.06.2019 18:53
> > > Subject: [EXTERNAL] Re: [eclipse-pmc] Review of mass changes
> > > Sent by: eclipse-pmc-bounces@xxxxxxxxxxx
> > >
> > >
> > >
> > > Hi,
> > >
> > > My 2c below ;)
> > >
> > > About !longChain.isEmpty() vs longChain.size() > 0, I favor the
> > > first one because isEmpty() is theorically a O(1) operation while
> > > size() is a O(n). Of course, most of smart enough implementations
> > > have this optimized and make size() a O(1), but there is usually no
> > > guarantee it is so. So size() is more expensive that isEmpty() and
> > > should be preferred.
> > > About readability, I understand the concern and I would like to
> > > suggest an alternative for that case: longChain.isEmpty() == false,
> > > which seems to have the qualities requested by all parties.
> > >
> > > About requiring a review for mass changes, +1.
> > > About not allowing mass change after some milestone, +1.
> > >
> > > Cheers
> > >
> > >
> > > --
> > > Mickael Istria
> > > Eclipse IDE developer, for Red Hat Developers
> > > _______________________________________________
> > > eclipse-pmc mailing list
> > > eclipse-pmc@xxxxxxxxxxx
> > > To change your delivery options, retrieve your password, or
> > > unsubscribe from this list, visit
> > > https://www.eclipse.org/mailman/listinfo/eclipse-pmc
> > >
> > > _______________________________________________
> > > eclipse-pmc mailing list
> > > eclipse-pmc@xxxxxxxxxxx
> > > To change your delivery options, retrieve your password, or
> > > unsubscribe from this list, visit
> > > https://www.eclipse.org/mailman/listinfo/eclipse-pmc
> > >
> > > _______________________________________________
> > > eclipse-pmc mailing list
> > > eclipse-pmc@xxxxxxxxxxx
> > > To change your delivery options, retrieve your password, or
> > > unsubscribe from this list, visit
> > > https://www.eclipse.org/mailman/listinfo/eclipse-pmc
> > >
> > > [attachment "graycol.gif" deleted by Daniel Megert/Zurich/IBM]
> > > _______________________________________________
> > > eclipse-pmc mailing list
> > > eclipse-pmc@xxxxxxxxxxx
> > > To change your delivery options, retrieve your password, or
> > > unsubscribe from this list, visit
> > > https://www.eclipse.org/mailman/listinfo/eclipse-pmc
> > >
> > >
> > > _______________________________________________
> > > eclipse-pmc mailing list
> > > eclipse-pmc@xxxxxxxxxxx
> > > To change your delivery options, retrieve your password, or
> > > unsubscribe from this list, visit
> > > https://www.eclipse.org/mailman/listinfo/eclipse-pmc
> > > _______________________________________________
> > > eclipse-pmc mailing list
> > > eclipse-pmc@xxxxxxxxxxx
> > > To change your delivery options, retrieve your password, or
> > > unsubscribe from this list, visit
> > > https://www.eclipse.org/mailman/listinfo/eclipse-pmc
> >
> > >
> > > _______________________________________________
> > > eclipse-pmc mailing list
> > > eclipse-pmc@xxxxxxxxxxx
> > > To change your delivery options, retrieve your password, or
> > > unsubscribe from this list, visit
> > > https://www.eclipse.org/mailman/listinfo/eclipse-pmc
> > >
> > > --
> > > Eclipse Platform project co-lead
> > > CEO vogella GmbH
> > >
> > > Haindaalwisch 17a, 22395 Hamburg
> > > Amtsgericht Hamburg: HRB 127058
> > > Geschäftsführer: Lars Vogel, Jennifer Nerlich de Vogel
> > > USt-IdNr.: DE284122352
> > > Fax (040) 5247 6322, Email: lars.vogel@xxxxxxxxxxx, Web: http://
> > > www.vogella.com_______________________________________________
> > > eclipse-pmc mailing list
> > > eclipse-pmc@xxxxxxxxxxx
> > > To change your delivery options, retrieve your password, or
> > > unsubscribe from this list, visit
> > > https://www.eclipse.org/mailman/listinfo/eclipse-pmc
> >
> > _______________________________________________
> > eclipse-pmc mailing list
> > eclipse-pmc@xxxxxxxxxxx
> > To change your delivery options, retrieve your password, or unsubscribe from this list, visit
> > https://www.eclipse.org/mailman/listinfo/eclipse-pmc
>
>
>
> --
> Eclipse Platform project co-lead
> CEO vogella GmbH
>
> Haindaalwisch 17a, 22395 Hamburg
> Amtsgericht Hamburg: HRB 127058
> Geschäftsführer: Lars Vogel, Jennifer Nerlich de Vogel
> USt-IdNr.: DE284122352
> Fax (040) 5247 6322, Email: lars.vogel@xxxxxxxxxxx, Web: http://www.vogella.com
> _______________________________________________
> eclipse-pmc mailing list
> eclipse-pmc@xxxxxxxxxxx
> To change your delivery options, retrieve your password, or unsubscribe from this list, visit
> https://www.eclipse.org/mailman/listinfo/eclipse-pmc
>
>
> [attachment "graycol.gif" deleted by Daniel Megert/Zurich/IBM] _______________________________________________
> eclipse-pmc mailing list
> eclipse-pmc@xxxxxxxxxxx
> To change your delivery options, retrieve your password, or unsubscribe from this list, visit
> https://www.eclipse.org/mailman/listinfo/eclipse-pmc
>
>
> _______________________________________________
> eclipse-pmc mailing list
> eclipse-pmc@xxxxxxxxxxx
> To change your delivery options, retrieve your password, or unsubscribe from this list, visit
> https://www.eclipse.org/mailman/listinfo/eclipse-pmc



-- 
Eclipse Platform project co-lead
CEO vogella GmbH

Haindaalwisch 17a, 22395 Hamburg
Amtsgericht Hamburg: HRB 127058
Geschäftsführer: Lars Vogel, Jennifer Nerlich de Vogel
USt-IdNr.: DE284122352
Fax (040) 5247 6322, Email: lars.vogel@xxxxxxxxxxx, Web: http://www.vogella.com


Back to the top