Re: LOG4J2-3260 Missing branch protection settings

2022-01-28 Thread Gary Gregory
I've said my bit, I'll wait for community consensus for now. Does changing any of this requires a VOTE and if s, under what veto rules? Gary On Fri, Jan 28, 2022, 10:26 Volkan Yazıcı wrote: > I totally agree with Carter's points. > > Given the current state of discussion, I propose the followin

Re: LOG4J2-3260 Missing branch protection settings

2022-01-28 Thread Ralph Goers
See below > On Jan 28, 2022, at 8:26 AM, Volkan Yazıcı wrote: > > I totally agree with Carter's points. > > Given the current state of discussion, I propose the following: > > - Every changes goes into a PR +1 > - Merge requires a successful CI build +1 > - Merge requires at least on

Re: LOG4J2-3260 Missing branch protection settings

2022-01-28 Thread Volkan Yazıcı
I totally agree with Carter's points. Given the current state of discussion, I propose the following: - Every changes goes into a PR - Merge requires a successful CI build - Merge requires at least one "approval" Here "approval" can be fulfilled by following means: - The committer h

Re: LOG4J2-3260 Missing branch protection settings

2022-01-27 Thread Ralph Goers
I have a feeling this is going to end up being a topic on our next video chat. I do appreciate your desire to use PRs for learning. I think you could do that even if approvals aren’t required. Having PR’s sit for a month before they can be committed is way too long. At my employer PRs are rarely

Re: LOG4J2-3260 Missing branch protection settings

2022-01-27 Thread Carter Kozak
Thank you for expanding upon this, Gary. The reasons I enjoy working on open-source projects are rooted in trust and community, but lead me to a different conclusion. Let me elaborate: At work I interact with a set of developers who have a great deal of experience working on our software, and ov

Re: LOG4J2-3260 Missing branch protection settings

2022-01-27 Thread Matt Sicker
I think I'd lean toward a CTR by default policy where larger changes should go through a PR. Simple cleanups, refactorings, and other easy things shouldn't require more friction than the CTR plus release vote process. I'd even suggest using PRs wherever you're not already fairly comfortable in the

Re: LOG4J2-3260 Missing branch protection settings

2022-01-27 Thread Ralph Goers
If I was asked to vote on moving to RTC I don’t know if I would be -1 but my vote would be negative. Largely for the same reason as Gary. Yes, stuff has been committed that I wish wasn’t but I can guarantee RTC wouldn’t have fixed that. It is simply that I couldn’t review the commit in a timely

Re: LOG4J2-3260 Missing branch protection settings

2022-01-27 Thread Gary Gregory
Crater and all, Let me further elaborate that one of the attractions here is the Apache value of community over code, and for me, commit-then-review, exemplifies that. Ironically, I know people here a lot less personally and professionally than I do people I work with; but, I trust you, I trust ev

Re: LOG4J2-3260 Missing branch protection settings

2022-01-27 Thread Gary Gregory
Hi Carter, I think we'll have to agree to disagree, especially if you want RTC just to add a single getter method, as your example shows. At work we use RTC for everything, no exceptions, and that's all good and works for our team, _at work_. This is not what I want to do in my free time. Gary O

Re: LOG4J2-3260 Missing branch protection settings

2022-01-26 Thread Carter Kozak
What if RTC only applied to the primary branch, release-2.x? We've had changes like this[1] which I discovered after the fact and wish we'd had a chance to discuss before it merged. Pushing changes prior to review is faster and easier for the committer, but ultimately creates an arduous process

Re: LOG4J2-3260 Missing branch protection settings

2022-01-26 Thread Gary Gregory
Not for me, this changes CTR to RTC, which is fine for work$, but too slow for me here. When I find time for FOSS, I just want to go and do it, not get bogged down in process. RTC is fine for a new medium to large feature, but not for everything. Right now, I do something in release-2.x, then cherr

Re: LOG4J2-3260 Missing branch protection settings

2022-01-26 Thread Matt Sicker
I'd be on board with this. It might be worth having some rule that if a committer's PR goes unreviewed for over a certain period of time, then the committer can just merge the PR after it passes the automated checks. I don't want this project's development to end up stalled because everyone is eith

Re: LOG4J2-3260 Missing branch protection settings

2022-01-26 Thread Carter Kozak
I'd love to start using the github PR workflow for our contributions. I've been using them for my changes for a while and find it much easier than running a full build locally to verify each change on my development system. While we've historically used "commit then review", I find it much easier

LOG4J2-3260 Missing branch protection settings

2022-01-26 Thread Volkan Yazıcı
According to the OSSF Scorecards , we are missing two check marks (LOG4J2-3260 ) there: 1. Require code review (every change goes into a PR and requires at least one reviewer) 2. Require a CI status chec