Hi.

2020-06-06 10:18 UTC+02:00, GitBox <g...@apache.org>:
>
> XenoAmess opened a new pull request #143:
> URL: https://github.com/apache/commons-math/pull/143
>
>
>    use System.arraycopy instead of loop.
>

There are several issues with this PR.

It contains several commits, among which some seem,
IIUC, to partly revert a previous change in that same PR.
It that situation, you must squash the commits (so that
the net changes stand out).
Also, the commit messages are uninformative ("refine",
"fix false positive").
The PR contains different types of changes: one is "use
arraycopy" but the others are not documented anywhere
(commit message, JIRA).
Traceability and easy reviewing are requirements for
"Commons" components.
For example, modifying the code that fills an array is
not a "minor" change, even if just because it makes the
reviewer wonder "Why?" (and the answer is nowhere to
be found from the official tracking tools).
If some performance improvement is unexpected from
looking at the code change, benchmark are indeed
necessary especially if the new code is less clear (and
a summary of the results should certainly make it into
the commit message and/or the corresponding JIRA
report).  [Also, an appropriately named utility method
is probably better than inline statements.]

Regards,
Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Reply via email to