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