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

Reply via email to