+1 With Herve fully here.
BUT I agree somewhat with Eliotte as well: We need to get rid of our habit of neglecting PRs. At least _some_ feedback on PR would be welcome, as leaving PRs hanging without ANY response is the worst way of treating contributors, and we do lack them. T On Sun, Feb 18, 2024 at 5:12 PM Hervé Boutemy <herve.bout...@free.fr> wrote: > FYI I reviewed PRs and merged what I was confident with > I can't say if this one can be merged: I don't know the feature > sufficiently > > I understand the good intent of the feedback, even if I find -1 a little > bit > strong: I did what I could, believe in my good intent too doing that > release > > Regards, > > Hervé > > Le dimanche 18 février 2024, 15:14:37 CET Elliotte Rusty Harold a écrit : > > On Sun, Feb 18, 2024 at 8:43 AM Romain Manni-Bucau > > > > <rmannibu...@gmail.com> wrote: > > > Hi Eliotte, > > > > > > Is the -1 only motivated by the fact there is a PR opened? > > > > That there is an unaddressed PR that looks worthy of review and has > > been open for months without a reply. 13 others are open. One of them > > is not ready for review. I haven't looked at the other 12 since, IMHO, > > one unreviewed PR is sufficient reason to delay a cut. > > > > > If so please consider two things: > > > * it is always the case for most releases (maven, asf, living projects > > > ;)) > > > so not sure when it became a hard criteria but if so we should probably > > > think to be more open if we want to release a day > > > > It isn't necessary to finish and merge all PRs before a release, but a > > review is not much to ask when someone has volunteered effort to fix > > something. Rarely, there's an emergency push for a critical bug. Short > > of that, it doesn't take all that long to scan the Github queue and > > see if there's anything useful that got missed. In practice though I > > often find even simple dependabot PRs unmerged after a release is cut. > > I would like to get in the habit of tidying up the queue prior to a > > release cut. > > > > > * If you take time to review the PR you mention you will also see it > just > > > revert a fix and that the final code should be more complex if it > lands in > > > shade plugin a day - there are always workarounds there - so don't > think > > > we > > > can make it in the week so can await another round and way more time to > > > make it right (long story sort shade has an old bug where it mixes the > > > same > > > config for different kind of rewritten sources, while it often works > and > > > stays simple, it also makes it insanely hard to be right since you > don't > > > know what you rewrite - a variable, a package, a class name, ...) > > > > That's all fine, but please say this on the PR so the new contributor > > can address these points. It's not good for PRs to sit in limbo for > > years. Commenting on PRs also helps to avoid someone else who doesn't > > have the context you do (e.g. me) from coming along and simply merging > > the PR because I don't happen to notice the problems you do. > > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org > For additional commands, e-mail: dev-h...@maven.apache.org > >
