[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [eclipse-pmc] Review of mass changes
- From: Tom Schindl <tom.schindl@xxxxxxxxxxxxxxx>
- Date: Tue, 18 Jun 2019 22:23:34 +0200
- Delivered-to: firstname.lastname@example.org
- User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:60.0) Gecko/20100101 Thunderbird/60.7.1
I'm not on the PMC but I'd like to provide my 2cents.
On Topic 1: I'm 100% with you that the current version is much more
On Topic 2: Once more i can only agree with you
Anyways this is just me, an ordinary (long term) contributor.
On 18.06.19 22:07, Andrey Loskutov wrote:
> Dear PMC,
> First topic.
> I was asked by Lars to ask PMC to help with the review of the change
> https://git.eclipse.org/r/#/c/144392/, especially
> The patch with the title "Use isEmpty instead of size" and the comment
> "Improves readability" suggests to change
> return getPossibleCandidates(constraint, exports, null, hook,
> true).size() > 0;
> return !getPossibleCandidates(constraint, exports, null, hook,
> In my experience such code where "not" is far away from the actual
> negatedÂ method call leads to misinterpretation of the code while
> reading, and therefore does not improve the code readability as claimed
> in the patch comment.
> It looks like Lars has different opinion on this topic, so he asked me
> to ask PMC about this.
> I personally think that I do not need to ask PMC every time I comment on
> some patch, but if Lars asks, I do, so here we are. I'm sorry for
> wasting your time, but it was not my idea to call PMC.
> Please review the patch as asked by Lars.
> Second topic.
> From my experience in the past years we had many mass-patches that
> broke something, and I had pleasure to fix them by myself. In most cases
> those mass patches were considered "harmless" and committers never asked
> anybody else for a review and just merged the patches.
> Now Lars started another huge patch series that are going across entire
> Eclipse platform, where not just some white space is changed, but where
> the real code (conditionals) are changed, and where very subtle bugs may
> appear. I have no time (and I don't want) to review all those mass
> changes, but someone should, because it is irresponsible to do something
> like this in such way.
> I would like to ask PMC here that from now on and for the future such
> mass changes that change real code (no white space etc) must have a
> mandatory +1 from aÂ second reviewer, to avoid regressions.
> Kind regards,
> Andrey Loskutov
> ÐÐÐÑÐÐÐÐ ÑÑÐÐÐÑÑÐÑ - ÐÐÐÐ ÑÑÐ ÑÐÐÐÑ ÑÑÐÐÐÑÑÐÑ
> eclipse-pmc mailing list
> To change your delivery options, retrieve your password, or unsubscribe
> from this list, visit
Tom Schindl, CTO
BestSolution.at EDV Systemhaus GmbH
Eduard-Bodem-Gasse 5-7. A-6020 Innsbruck
Reg. Nr. FN 222302s am Firmenbuchgericht Innsbruck