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