I thought it was obvious so I didn't do the benchmark. You need it so I've done it now. pinned at https://github.com/apache/commons-lang/pull/565
Gilles Sadowski <gillese...@gmail.com> 于2020年7月26日周日 下午11:48写道： > 2020-07-26 17:34 UTC+02:00, Xeno Amess <xenoam...@gmail.com>: > > Thanks for the suggestions about JIRA message and commit style. > > Will find some time to refine the texts. > > > >> ... randomly picking LANG-1576 (sorry if the others don't fit the > >> following), I'll stress again that there are more important things > >> to do before such (supposed) performance enhancement. > >> These are changes which a committer might do, but that are not > >> worth a reviewer's time unless it comes with benchmarks that > >> prove the claim (see for example the work done by Alex to > >> squeeze out the last drops of performance in "Commons RNG"). > > > > I do not quite agree, as this is a base library, not a software. > > If it be a software, yes, unless we meet bottle-neck we should not > > over-optimize. > > But this is a base library, so IMO we should squeeze out as much > > performance as we can, everywhere, because we actually cannot make sure > > which function could be widely used by who. > > I specifically commented on LANG-1576. > Where is the benchmark? > > Gilles > > > > >> For example, following Gary's and Bruno's comments, you > >> could set up a branch that would delete all the deprecated > >> codes > > > > I think that should be done just before we make lang 4.0. > > But as far as I know, lang 4.0 is still far away, at least several months > > time later, so I don't think this is the right time to take action to > > remove the deprecated codes... > > Means maintaining such a branch for several months seems not quite > worthy. > > > >> and look for further code bloat that could be removed from the next > major > > release > > > > I will if I see any. > > > > > > Gilles Sadowski <gillese...@gmail.com> 于2020年7月26日周日 下午11:13写道： > > > >> Hi Xeno. > >> > >> 2020-07-26 13:10 UTC+02:00, Xeno Amess <xenoam...@gmail.com>: > >> >>> For examples about my prs at commons-lang,if my memory is correct, > >> >>> only > >> >>> gary (and sometimes kinow) reviewed my prs, and I don't think we > have > >> > only > >> >>> two committers in commons-lang. > >> > > >> >> Are there JIRA reports? > >> > > >> > My log here is: > >> > > >> > LANG-1545 merged by gary > >> > LANG-1561 merged by gary > >> > LANG-1563 merged by gary > >> > LANG-1562 merged by gary > >> > LANG-1564 merged by gary > >> > LANG-1560 merged by gary > >> > LANG-1552 merged by gary > >> > LANG-1553 merged by gary > >> > LANG-1554 merged by gary > >> > LANG-1555 merged by gary > >> > LANG-1558 merged by gary > >> > LANG-1559 merged by gary > >> > LANG-1556 merged by gary > >> > LANG-1565 merged by gary > >> > LANG-1557 merged by gary > >> > LANG-1546 merged by kinow > >> > LANG-1549 merged by chtompki > >> > >> As said, thanks for your interest in "Commons" but... > >> > >> > pending: > >> > 10+ sub-quests in LANG-1573 pending, for near 1 month already. most of > >> > which is performance refines in StringUtils. > >> > >> ... randomly picking LANG-1576 (sorry if the others don't fit the > >> following), I'll stress again that there are more important things > >> to do before such (supposed) performance enhancement. > >> These are changes which a committer might do, but that are not > >> worth a reviewer's time unless it comes with benchmarks that > >> prove the claim (see for example the work done by Alex to > >> squeeze out the last drops of performance in "Commons RNG"). > >> > >> Also, please be more informative in the title of the reports: > >> "refine <something>" says that you changed <something> > >> but not how or why. Then by going to the JIRA report itself, we > >> don't get more information; the "description" field should > >> contain a description, not just the link to the PR. > >> So a potential reviewer, instead of getting a direct hint about > >> whether he could have the knowledge or interest in committing > >> the changes, must go all the way (link in the notification mail, > >> link in JIRA, often multiple links in the PR's GH page) to the diff > >> itself, to figure out that in LANG-1576, you changed "if ... else" > >> statements to a "switch" statement. > >> There are indeed 2 commits there (instead of 1 as I've stressed > >> several times already) and that just increases the review time > >> because: > >> * I click on the last one and I don't see the diff with "master" > >> but only the diff wrt your changes. > >> * Then I click on the first commit and and wonder: Is it OK to > >> have a fall-through there? > >> * Then I go back to the list of commit and wonder: What was > >> the CheckStyle issue? > >> * Then I click again the last commit and see that there is now > >> a duplicate a statement and wonder: Is that necessary[2], or > >> is it cutting corners to prevent the CheckStyle warning and > >> let Travis green-light the change? > >> * Then I figure out that I cannot take the responsibility to > >> make the commit because the improvement is not obvious. > >> > >> > I'm not requesting somebody must review my pr now or something. > >> > And I know committers are busy. > >> > I say this just for showing, in my view, we really have no enough > >> > reviewers. > >> > And if somebody has any ideas about how we can solve this, by making > >> > more > >> > reviewers or making current non-active committers more active, or > other > >> > more advice... > >> > >> You can help the project by taking on the suggestion which > >> I've already made, and that amounts to increasing the ratio > >> of contribution time to review time. > >> For example, following Gary's and Bruno's comments, you > >> could set up a branch that would delete all the deprecated > >> codes, and look for further code bloat that could be removed > >> from the next major release, and ensure that alternatives are > >> working and advertised in the Javadoc and release notes. > >> > >> Regards, > >> Gilles > >> > >> [1] https://issues.apache.org/jira/browse/LANG-1576 > >> [2] > >> > https://checkstyle.sourceforge.io/apidocs/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheck.html > >> > >> >> [...] > >> > >> --------------------------------------------------------------------- > >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > >> For additional commands, e-mail: dev-h...@commons.apache.org > >> > >> > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > >
