commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles <>
Subject Re: [5/7] [math] Fix "FastMath#round(..)" to comply to changed contract of "Math#round()" in Java 8
Date Mon, 08 Aug 2016 14:22:27 GMT
On Mon, 8 Aug 2016 13:14:18 +0200, Emmanuel Bourg wrote:
> Le 8/08/2016 à 00:42, Gilles a écrit :
>> In CM, the usage was to not use "import static".
> You should make an exception for the unit tests, otherwise the JUnit 
> 4
> syntax is too verbose (and JUnit 4 was designed with static imports 
> in
> mind).

There are pro and contra; IMO, saving a few characters is not worth
wondering upon reading whether "assertEquals" is from (JUnit) "Assert"
or Commons Math "TestUtils".

>> Excerpts from "":
>> + Create a topic branch from where you want to base your work (this 
>> is
>> usually the develop/trunk branch).
> This is too complicated for simple changes. Topic branches are
> interesting for long lived branches involving many modifications 
> needing
> review. Forcing a topic branch for any modification is a hassle.

One big advantage of a branch is that reviewers can examine different
"topics" at their will (e.g. out of order).

"Simple" changes are those that do not involve any build risks (such as
a Jenkins failure).  So, mostly: documentation upgrades.

> If you want Commons Math to be welcoming to new contributors you 
> should
> consider relaxing the workflow constraints.

When people get accustomed to git, they won't see it as a constraint,
rather as second nature.

Much better to discuss and modify code when it is in a topic branch
rather than after it was merged into the main one.

I think that it is also much easier to review (by making a diff between
branches rather than search for the appropriate commit ids).

>> If there is a JIRA report, the log must indeed make it 
>> straightforward
>> to find it.
> I agree, but is this case I noticed the issue in JIRA after pushing 
> the
> changes.

If performed in a topic branch, the questions prompted by the changes
would have offered the opportunity to add the information in the merge

QED. ;-)


> Emmanuel Bourg

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message