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 
Another observation:
In the recent past, I frequently had to wait long time before my bug fix was built on jenkins, because lots of cleanup jobs jammed the build queue.
With one build taking around 1 hour (please don't complain, this is 1 hour spent well on lots of tests), each build potentially slows down productive development. Haven floods of up-to ten cleanup builds is not nice. 

Stephan

On 25.06.19 09:44, Lars Vogel wrote:

Jay, I also suggest to set unnecessary import to error for JDT core to
prevent this warning from happen. Gerrit validation should also
consider these settings.

Please comment in https://bugs.eclipse.org/bugs/show_bug.cgi?id=548606

Best regards, Lars

On Tue, Jun 25, 2019 at 8:30 AM Lars Vogel <lars.vogel@xxxxxxxxxxx> wrote:


Jay, AFAIK where is a pending patch from Alex for JDT core which would allow to fail the Gerrit if new warnings are introduced.

https://git.eclipse.org/r/#/c/142994/

Please review.

Best regards, Lars

Jayaprakash Arthanareeswaran <jarthana@xxxxxxxxxx> schrieb am Di., 25. Juni 2019, 07:23:


Here's another example of what else can go wrong. One of the recent change left a new warning in the code:
https://download.eclipse.org/eclipse/downloads/drops4/I20190624-1800/compilelogs/plugins/org.eclipse.jdt.core_3.18.100.v20190624-0139/@dot.html#OTHER_WARNINGS

https://bugs.eclipse.org/bugs/show_bug.cgi?id=548505

Not a big deal, one might say. But it costs someone else little bit of time. Basically, it underlines the fact that the reviewer has to make sure that the patch is pulled (and not just glance over the changes on Gerrit) and make sure that the code base is as clean as before the patch

Regards,
Jay

----- Original message -----
From: "Manoj Palat" <manoj.palat@xxxxxxxxxx>
Sent by: eclipse-pmc-bounces@xxxxxxxxxxx
To: eclipse-pmc@xxxxxxxxxxx
Cc:
Subject: [EXTERNAL] Re: [eclipse-pmc] Review of mass changes
Date: Mon, Jun 24, 2019 9:42 AM


Hi Dani,
I guess I needed to make it more clear here.


So, the sequence would be: commit the change to BETA *before* merging?

Definitely not.
What I meant was to have a gerrit - "then the author himself creating a gerrit (after conflict resolutions, if any) for the BETA_JAVAn branch."

This is just a verification step - This would make sure that the merge is smooth and we don't have any additional test failures when the mass changes get merged with BETA branch. The merging itself will happen from the master later - with no special consideration being required for this change - ie whenever we merge master to BETA, this change will also get merged.

Regards,
Manoj.

eclipse-pmc-bounces@xxxxxxxxxxx wrote on 06/21/2019 08:10:29 PM:


From: "Daniel Megert" <daniel_megert@xxxxxxxxxx>
To: eclipse-pmc@xxxxxxxxxxx
Date: 06/21/2019 08:10 PM
Subject: [EXTERNAL] Re: [eclipse-pmc] Review of mass changes
Sent by: eclipse-pmc-bounces@xxxxxxxxxxx

The question was directed to Manoj. Sorry if that was not clear.

Dani



From:        Lars Vogel <lars.vogel@xxxxxxxxxxx>
To:        eclipse-pmc@xxxxxxxxxxx
Date:        21.06.2019 16:32
Subject:        [EXTERNAL] Re: [eclipse-pmc] Review of mass changes
Sent by:        eclipse-pmc-bounces@xxxxxxxxxxx




So, the sequence would be: commit the change to BETA *before* merging?


I would argue that this should be a decision the committer who does
the code review.

Given that JDT is frequently not fast in review feedback and rejects
certain patches, requiring this work upfront seems bad for growing
more community patches in this area.

Best regards, Lars

On Fri, Jun 21, 2019 at 4:22 PM Daniel Megert
<daniel_megert@xxxxxxxxxx> wrote:



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


So, the sequence would be: commit the change to BETA *before* merging?

An important thing is to do master and BETA very close to each

other to avoid that the change gets conflicts again.


Dani



From:        "Manoj Palat" <manoj.palat@xxxxxxxxxx>
To:        eclipse-pmc@xxxxxxxxxxx
Date:        21.06.2019 06:36
Subject:        [EXTERNAL] Re: [eclipse-pmc] Review of mass changes
Sent by:        eclipse-pmc-bounces@xxxxxxxxxxx
________________________________



@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.


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

[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
_______________________________________________
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

Back to the top