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

> Project leads monitor the Gerrit queue so that they can proactively help with community reviews.

> I agree, but let's not abuse this thread for that topic.

This was just a clarification to your statement "This way they can
react on a Gerrit change before it gets merged." which sounded to me
that projects should primary monitor the queue to block merges .

I'm glad we aware on the responsibility of the project leads to help
with community reviews.

Best regards, Lars

On Wed, Aug 14, 2019 at 2:42 PM Daniel Megert <daniel_megert@xxxxxxxxxx> wrote:
>
> > Project leads monitor the Gerrit queue so that they can proactively help with community reviews.
>
> I agree, but let's not abuse this thread for that topic.
>
> BTW, it's also good for committers to monitor the Gerrit queue of their projects.
>
> Dani
>
>
>
> From:        Lars Vogel <lars.vogel@xxxxxxxxxxx>
> To:        eclipse-pmc@xxxxxxxxxxx
> Date:        14.08.2019 14:35
> Subject:        [EXTERNAL] Re: [eclipse-pmc] Review of mass changes
> Sent by:        eclipse-pmc-bounces@xxxxxxxxxxx
> ________________________________
>
>
>
> > That way they can react on a Gerrit change before it gets merged.
>
> I would phrase this different:
>
> Project leads monitor the Gerrit queue so that they can proactively
> help with community reviews.
> IMHO projects leads are supposed to help proactively with incoming
> community patches. That is IMHO a very important part of being a
> leader.
>
> Best regards, Lars
>
> Best regards Lars
>
> On Wed, Aug 14, 2019 at 2:29 PM Aleksandar Kurtakov <akurtako@xxxxxxxxxx> wrote:
> >
> >
> >
> > On Wed, Aug 14, 2019 at 3:26 PM Daniel Megert <daniel_megert@xxxxxxxxxx> wrote:
> >>
> >> The PMC also agreed that a committer can block a mass change (like any other change) with -1 if there are good arguments. That's why the following point is important:
> >
> >
> > "if there are good arguments" is important - just saying "-1, we are post M1" is not enough.
> >
> >>
> >>
> >>
> >> > Project leads should monitor Gerrit queues
> >>
> >> That way they can react on a Gerrit change before it gets merged.
> >>
> >> Dani
> >>
> >>
> >>
> >> From:        Lars Vogel <lars.vogel@xxxxxxxxxxx>
> >> To:        eclipse-pmc@xxxxxxxxxxx
> >> Date:        14.08.2019 13:24
> >> Subject:        [EXTERNAL] Re: [eclipse-pmc] Review of mass changes
> >> Sent by:        eclipse-pmc-bounces@xxxxxxxxxxx
> >> ________________________________
> >>
> >>
> >>
> >> Hi Sarika,
> >>
> >> The PMC discussed that topic yesterday and we decided:
> >>
> >> Resolution:
> >>
> >> Mass changes are requested not to be more than 50 files in general if they can be split up but ...
> >> Don't force it to be split for cases it is difficult to do so
> >> Don't restrict mass changes to specific milestones
> >> Cleanups are allowed at any time during development cycle
> >> Should reviewers be added??
> >>
> >> No adding reviewers is not required by committers
> >> Project leads should monitor gerrit queues - PMC agreed
> >>
> >>
> >> Please see https://wiki.eclipse.org/Eclipse/PMC
> >>
> >> Best regards, Lars
> >>
> >> On Wed, Aug 14, 2019 at 1:17 PM Sarika Sinha <sarika.sinha@xxxxxxxxxx> wrote:
> >> As agreed by many people on this thread, I would like to reiterate the point that we should not engage in unnecessary cleanups after M1 in any component. Not limited to JDT as we have seen a change in platform https://git.eclipse.org/r/#/c/144099/cancause regression in other dependent components.
> >>
> >> I recently see some active cleanup changes in the last development week of M3:
> >> https://git.eclipse.org/r/#/c/147411/
> >> https://git.eclipse.org/r/#/c/144540/
> >> https://git.eclipse.org/r/#/c/147702/
> >>
> >> Merge could have been easily postponed to the next release.
> >>
> >> Thanks & Regards,
> >> Sarika
> >>
> >>
> >>
> >>
> >>
> >> From:        "Sarika Sinha" <sarika.sinha@xxxxxxxxxx>
> >> To:        eclipse-pmc@xxxxxxxxxxx
> >> Date:        20/06/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
> >>
> >>
> >> _______________________________________________
> >> 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[attachment "noname" 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
> >
> >
> >
> > --
> > Alexander Kurtakov
> > Red Hat Eclipse Team
> > _______________________________________________
> > 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