Alright, done. This pr is now splitted into two prs. https://github.com/apache/commons-math/pull/144 https://github.com/apache/commons-math/pull/143
Thx. Xeno Amess <[email protected]> 于2020年6月6日周六 下午7:47写道： > Yes you are right. > >you must squash the commits. > >the commit messages are uninformative > > Usually I'd do them when commiter think this pr is ok, and then tell me we > should merge it, and at that moment I squash it, and it get merged. > the commit messages will be fixed at that time too. > > >partly revert a previous change > > Yes, as my newest performance test shows something is not quite right in > my previous changes. It makes the codes even slower. > So I changed that part. > > >benchmark are indeed necessary > > agreed, I did a benchmark yesterday, and pointed a link on github pages. > I'll copy/paste > > >The PR contains different types of changes: one is "use arraycopy" but > the others are not documented anywhere (commit message, JIRA). > Ah, I forgot to create a jira ticket for it. sorry about that. > > So, in conclusion, I will close this pr, and make two jira tickets, for > the two different types of refines. > > Thanks for your help. > > Gilles Sadowski <[email protected]> 于2020年6月6日周六 下午7:33写道： > >> Hi. >> >> 2020-06-06 10:18 UTC+02:00, GitBox <[email protected]>: >> > >> > 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: [email protected] >> For additional commands, e-mail: [email protected] >> >>
