The quick turnaround time for PR reviews is important when I only have time to work on the project one or two days a week.
> On Sep 18, 2024, at 08:38, Ralph Goers <ralph.go...@dslextreme.com> wrote: > > I sort of agree and sort of not. It would be nice to have automation that can > enforce some rules. You can’t do that if there are none. > > We could agree in principle to a small number of files and lines but set the > enforcement to a larger number to allow for some exceptions. For example, if > we agreed on 10 files max with no more than 5 lines each (50 lines total) we > could set the enforcement at 20 files with a max of 200 lines (notice that is > not 10 lines per file). > > Ralph > >> On Sep 18, 2024, at 2:17 AM, Gary Gregory <garydgreg...@gmail.com> wrote: >> >> This proposal is two pages (on my phone)! I can't imagine counting lines >> and walking through these steps, it's crazy making IMO. How would you count >> lines anyway? From diff output? >> >> I'd rather skip the bikeshedding and let devs create PRs when they think it >> makes sense. IOW, when they want a review. Otherwise, I trust devs to check >> their work. >> >> Gary >> >> On Wed, Sep 18, 2024, 3:46 AM Volkan Yazıcı <vol...@yazi.ci.invalid> wrote: >> >>> I agree in principle, but... >>> >>> *Exemption criteria* >>> >>> I agree with Ralph on that we should specify criteria on what shall be >>> exempt from this PR-driven workflow policy. We should of course materialize >>> these criteria collaboratively. Below is my attempt for a starting point: >>> >>> Changes can be exempt from this policy if and only if they suffice *only >>> one* of the following conditions: >>> >>> 1. Grammatical corrections to the documentation (incl. Javadoc and >>> release notes) >>> 2. Code typo fixes¹ less than 10 LoC >>> 3. Infrastructure fixes¹ touching to `pom.xml` or CI scripts, and less >>> than 10 LoC >>> >>> ¹ A "fix" is assumed to not introduce a change in the expected behaviour of >>> the associated component. Changing the expected behaviour does not qualify >>> a fix. >>> >>> *Scoped repositories* >>> >>> I suggest extending the scope of this policy to cover the following >>> repositories too: >>> >>> 1. `logging-parent` >>> 2. `logging-log4j-jakarta` >>> 3. `logging-jmx-gui` >>> 4. `logging-samples` >>> 5. `logging-site` >>> 6. `logging-log4net-site` >>> >>> *Maintainer availability* >>> >>> The PR-driven workflow can fly because we have full-time maintainers. But >>> that will not be the case anymore in 2-3 months due to funds drying out. I >>> suspect introducing such a policy with strict deadlines (e.g., 24 hours is >>> a pretty tight time budget for a project with no full-time maintainers) and >>> no exemptions might jeopardize the streamlining of development in the long >>> run. That said, as Christian noted, we can adapt/drop the policy later on >>> if needed. >>> >>> I disagree with Matt's *"losing momentum from waiting for an unnecessary >>> code review"* statement. In the last three years or so – even before STF >>> funding and Piotr! – well-scoped PRs, where the author onboards reviewers >>> with sufficient navigation tips, have been processed very swiftly, AFAICT. >>> >>> On Wed, Sep 18, 2024 at 12:16 AM Ralph Goers <ralph.go...@dslextreme.com> >>> wrote: >>> >>>> This isn’t a vote so I am not going to. If I had to vote I wouldn’t vote >>>> for a policy that requires RTC always. However, I would vote for a policy >>>> that requires RTC when specified criteria are met. >>>> >>>> Ralph >>>> >>>>> On Sep 17, 2024, at 10:28 AM, Ralph Goers <ralph.go...@dslextreme.com> >>>> wrote: >>>>> >>>>> First, the obvious. I haven’t committed much in a while. The last >>>> several I did I used PRs primarily because it makes it easier for people >>> to >>>> review the changes but I didn’t necessarily wait for a review. For >>> really >>>> simple stuff I've never use a PR. However, with the switch from Jira to >>>> GitHub issues it might make more sense to use a PR since you have to have >>>> something to track the problem. But if there is already an issue then I >>>> would probably just use that. Again, depending on what is being done. >>>>> >>>>> I wouldn’t classify any of the work I’ve seen you doing recently as >>>> trivial or minor, so you only doing PRs makes sense. >>>>> >>>>> Ralph >>>>> >>>>>> On Sep 17, 2024, at 7:52 AM, Piotr P. Karwasz < >>> piotr.karw...@gmail.com> >>>> wrote: >>>>>> >>>>>> Hi Ralph, >>>>>> >>>>>> On Tue, 17 Sept 2024 at 15:47, Ralph Goers < >>> ralph.go...@dslextreme.com> >>>> wrote: >>>>>>> >>>>>>> Why? i.e. - what currently isn’t working? >>>>>> >>>>>> I merely wish to formalize what is already happening and set up a >>>>>> branch protection rule to enforce it. >>>>>> >>>>>> Note that I have never seen a PR in Log4Net being merged without a >>>> review. >>>>>> >>>>>> On the other hand you can probably find a couple of my recent PRs that >>>>>> don't have a formal review. Sure, I must have certainly consulted with >>>>>> someone on Slack before I merged them, but there is no sign of it. >>>>>> Let's make it formal, so users also see it. >>>>>> >>>>>> Piotr >>>>> >>>> >>>> >>> >
