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. > 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 > >
