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

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