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ı <[email protected]> wrote: > 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 himself (that is, in practice, no approval/review is > needed) > - By required reviewers + (optional) time-window (on timeout, issuer can > approve himself and get it merged) > > Is this something we can all agree on? > > Gary, if I am not mistaken, all this approach asks from you in addition to > what you are already doing is PR + CI checks. Is that a compromise you can > make? > > Ralph, I have read your remark about waiting for the next online meeting, > but that is on Feb 27, and I think we better shouldn't wait for a month to > get this rolling. > > On Thu, Jan 27, 2022 at 11:03 PM Carter Kozak <[email protected]> wrote: > > > 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 over time grow to adopt similar > > sets of biases. Working on open-source projects gives me the opportunity > to > > learn from a much broader set of strong, highly-motivated individuals > with > > a more diverse set of backgrounds. Asking questions on PRs is an > invaluable > > tool to learn things that I wouldn't otherwise discover, as well as to > > ensure that my mental model of the project is based on current > information. > > > > The eyes of the world are on log4j right now[1], and it's on us to take > > ownership of security given the criticality of the project. We should > > commit to radical incrementalism and experiment with different controls > and > > processes to move the needle on security while balancing the needs of the > > project. For the first step, I propose that we enable branch protections > on > > release-2.x and require at least one approval before merge. We can > > time-bound this to a month and then re-evaluate based on our experience. > We > > have the unique opportunity to lead open source efforts in supply chain > > security[2][3], and we should take full advantage of it. > > > > Carter > > > > 1. > > > https://www.whitehouse.gov/omb/briefing-room/2022/01/26/office-of-management-and-budget-releases-federal-strategy-to-move-the-u-s-government-towards-a-zero-trust-architecture/ > > 2. > > > https://www.whitehouse.gov/briefing-room/presidential-actions/2021/05/12/executive-order-on-improving-the-nations-cybersecurity/ > > 3. > > > https://www.whitehouse.gov/briefing-room/statements-releases/2022/01/13/readout-of-white-house-meeting-on-software-security/ > > > > On Thu, Jan 27, 2022, at 14:36, Matt Sicker wrote: > > > 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 codebase. For example, if I want to contribute > > > changes to JsonTemplateLayout, I'd likely want to make a PR so I could > > > get Volkan to review it. I'd also suggest using PRs wherever you > > > desire more explicit feedback from the team. > > > > > > Possibly unrelated, but I think if we fixed the triple-notification > > > issue when commits come in so that we only see the one email, then > > > that would make it easier to review code as it's committed. I don't > > > think I could be convinced to fully switch to RTC, though I could see > > > that being useful for legacy branches (e.g., backport fixes for the > > > Java 6 and Java 7 branches, and maybe even the release-2.x branch as > > > we get closer to 3.0). > > > > > > On Thu, Jan 27, 2022 at 9:49 AM Ralph Goers < > [email protected]> > > wrote: > > > > > > > > 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 > > manner. > > > > That would happen whether it is RTC or CTR. > > > > > > > > OTOH. If you want to move to a policy where every commit has to be > done > > > > through a PR that doesn’t require any approvals I could probably go > > for that. > > > > That would leave a record in GitHub that is easier to wade through > and > > > > eview than email is. It also isn’t much harder to do. It could also > > have the > > > > advantage of running a build against it through CI tooling. I would > > also be > > > > in favor of saying merges should be prevented until a successful > build > > of > > > > the PR is completed. I have found too many times that people are > > > > committing stuff and not doing a full rebuild so this would ensure > > that doesn’t > > > > happen. And the extra time a CI build would take doesn’t seem like > too > > > > much of a burden to me. > > > > > > > > In short, the only part about this I don’t like is having to wait for > > a review. > > > > We all pretty much know the difference between something that should > > > > be reviewed and something that doesn’t require it and follow that > > practice. > > > > > > > > Ralph > > > > > > > > > > > > > > > > > On Jan 27, 2022, at 7:16 AM, Gary Gregory <[email protected]> > > wrote: > > > > > > > > > > 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 > > > > > everyone on the PMC. We can make mistakes, we fix them, we can > revert > > > > > commits, and so on, and it is through all of these activities that > > we build > > > > > a community, our community. For me, review-then-commit, says "I > > don't trust > > > > > you". It feels like work. At that point, we might as well start our > > own > > > > > company and provide paid support and development for Log4j forks or > > > > > whatever else we want, then we can put in all the processes anyone > > wants. > > > > > > > > > > Gary > > > > > > > > > > On Thu, Jan 27, 2022 at 9:02 AM Gary Gregory < > [email protected]> > > wrote: > > > > > > > > > >> 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 > > > > >> > > > > >> On Wed, Jan 26, 2022 at 8:01 PM Carter Kozak <[email protected]> > > wrote: > > > > >> > > > > >>> 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 for reviewers. I've found it harder to have retroactive > > > > >>> conversations about changes that have already merged. > > > > >>> > > > > >>> -ck > > > > >>> > > > > >>> 1. > > https://lists.apache.org/thread/qn9b22fqv4yzg6jb114ovso40fnc1k0n > > > > >>> > > > > >>> On Wed, Jan 26, 2022, at 16:22, Gary Gregory wrote: > > > > >>>> 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 > > > > >>>> cherry-pick to master, already a PITA because of the JPMS mess, > > now it > > > > >>> has > > > > >>>> to be a 4 step process instead of 2? Pass. > > > > >>>> > > > > >>>> Gary > > > > >>>> > > > > >>>> > > > > >>>> On Wed, Jan 26, 2022, 15:25 Volkan Yazıcı <[email protected]> > wrote: > > > > >>>> > > > > >>>>> According to the OSSF Scorecards < > > https://github.com/ossf/scorecard>, > > > > >>> we > > > > >>>>> are missing two check marks (LOG4J2-3260 > > > > >>>>> <https://issues.apache.org/jira/browse/LOG4J2-3260>) there: > > > > >>>>> > > > > >>>>> 1. Require code review (every change goes into a PR and > > requires at > > > > >>>>> least one reviewer) > > > > >>>>> 2. Require a CI status check > > > > >>>>> > > > > >>>>> Even though I admit with the convenience of freedom we have > right > > > > >>> now, I > > > > >>>>> personally find it difficult to keep track of what is going in > > and > > > > >>> out. > > > > >>>>> This convention does not aim to obstruct the development > > progress, but > > > > >>>>> rather improve the overall code quality and spread the know-how > > in a > > > > >>>>> scalable way. Hence, I want to implement this feature on > > > > >>> `release-2.x` and > > > > >>>>> `master` branches. Thoughts? > > > > >>>>> > > > > >>>> > > > > >>> > > > > >> > > > > > > > > > >
