Skip to content

Conversation

ppkarwasz
Copy link
Contributor

Following the PMC discussion in September 2024, this PR changes the branch protection rules for the logging-log4j2 repository:

  • It requires all changes to 2.x and main to be submitted as PRs.
  • It requires all PRs to have at least one positive review to be merged.
  • It requires all conversations to be marked as resolved, before a PR is merged.

The current features offered by GitHub do not allow introducing exceptions on the size of the PR or other criteria, so let's try to review 100% of the PRs. Smaller teams manage to do it, so we should be able too.

Following the [PMC discussion in September 2024](https://lists.apache.org/thread/6gbos0rn3k4y3wjb1hcgnnols4ogqckl), this PR changes the branch protection rules for the `logging-log4j2` repository:

- It requires all changes to `2.x` and `main` to be submitted as PRs.
- It requires all PRs to have at least one positive review to be merged.
- It requires all conversations to be marked as resolved, before a PR is merged.

The current features offered by GitHub do not allow introducing exceptions on the size of the PR or other criteria, so let's try to review 100% of the PRs. Smaller teams manage to do it, so we should be able too.
@jvz
Copy link
Member

jvz commented Apr 1, 2025

I'm ok with this change for now. I will be quite loud about it, however, if it ends up being an issue in the future, but I hope that won't be the case!

@ppkarwasz
Copy link
Contributor Author

If you make small PRs and add two reviewers (e.g., @vy and me), it shouldn't be a problem.
For bigger changes you can use a feature branch and make smaller PRs into it.
For micro (e.g. Javadoc or final modifier) changes, you can make a draft PR and review it, when there are more changes.

@jvz
Copy link
Member

jvz commented Apr 2, 2025

It won't end up being related to PR size or even making PRs. I would be annoyed if my PRs sit unreviewed.

@ppkarwasz
Copy link
Contributor Author

If we manage to learn how to use GitHub Projects (in the "Projects" tab, currently team-private), we can keep track of the order in which PRs are waiting for a review.

@ppkarwasz
Copy link
Contributor Author

I modified this PR according to the vote on dev@logging.

Can you check if the values look good to you? I have posted an initial JSON schema of .asf.yaml as apache/infrastructure-asfyaml#62. It is pretty much work in progress, but it should cover our usage.

@ppkarwasz ppkarwasz merged commit 824e921 into 2.x Apr 11, 2025
11 checks passed
@ppkarwasz ppkarwasz deleted the feature/rtc branch April 11, 2025 09:25
@github-project-automation github-project-automation bot moved this from In review to Done in Log4j bug tracker Apr 11, 2025
@ppkarwasz
Copy link
Contributor Author

A couple of additional commits (some expected, some unexpected), were required to get this working:

  • 3052840: the .asf.yaml processor might fail on null
  • f6db7c7: the processor definitively does not support YAML references
  • eefcf0b: a typo in the specification

@ppkarwasz
Copy link
Contributor Author

As usual with GitHub configuration, a couple more commits where required:

  • Due to an error in the context names of the checks, GitHub was waiting for non existing checks. To unbrick it I pushed 9f3c495 via gitbox.apache.org.
  • The context names were tested on main in 1798fc2.
  • With 514537d the process should be finished and I copied the configuration tested on main to 2.x.

@grobmeier
Copy link
Member

+1. Support for this, even when it is already merged.

ppkarwasz added a commit to apache/logging-log4j-tools that referenced this pull request May 29, 2025
This change enables the same branch protection that was implemented in `logging-log4j2`.

See apache/logging-log4j2#3582 and apache/logging-log4j2#3662
ppkarwasz added a commit to apache/logging-log4j-tools that referenced this pull request Jun 3, 2025
This change enables the same branch protection that was implemented in `logging-log4j2`.

See apache/logging-log4j2#3582 and apache/logging-log4j2#3662
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants