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