Skip to content

Document what is required before merging a pull request - #5065

Open
psss wants to merge 2 commits into
mainfrom
psss-github-rulesets
Open

Document what is required before merging a pull request#5065
psss wants to merge 2 commits into
mainfrom
psss-github-rulesets

Conversation

@psss

@psss psss commented Jul 17, 2026

Copy link
Copy Markdown
Member

Clearly document what is required and what is recommended before merging a pull request. Also include the github rulesets exports so that the configuration can be easily shared/updated across all teemtee repositories using import a new ruleset. For the required checks the pick-the-required-checks placeholder is needed to actually export the settings. To be adjusted individually for each repository.

Pull Request Checklist

  • write the documentation

Clearly document what is required and what is recommended before
merging a pull request. Also include the github rulesets exports
so that the configuration can be easily shared/updated across all
teemtee repositories using import a new ruleset.
@psss psss added this to planning Jul 17, 2026
@psss psss moved this to review in planning Jul 17, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds GitHub branch protection rulesets (enforced.json and override.json) to enforce merge requirements like linear history, squash merging, and resolved conversations, while allowing release leads to override them. It also updates docs/contribute.rst to document these policies. Feedback recommends removing repository-specific metadata (such as id, source, and source_type) from the JSON configurations to make them generic and reusable.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread .github/settings/enforced.json Outdated
@psss psss added documentation Improvements or additions to documentation area | process The way how we collaborate on the development labels Jul 17, 2026
@psss

psss commented Jul 17, 2026

Copy link
Copy Markdown
Member Author

Docs check is expected to fail as the new links are pointing to files in the future main branch.

@LecrisUT

Copy link
Copy Markdown
Member

Docs check is expected to fail as the new links are pointing to files in the future main branch.

Did we not add something to construct links to local files more consistently? If not can make an issue for it, I should at least have the snippet from some other projects.

@LecrisUT LecrisUT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the footgun of replacing required_status_checks with something empty, LGTM

Comment on lines +20 to +24
"required_status_checks": [
{
"context": "pick-the-required-checks"
}
]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should just add the full set here. It would make bumping it more automatic. Each repo would have a different set, but maybe we can merge it with another source, maybe even something fmf-y?

My concern here is that when we use an import, it would override it with the bogus value here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should just add the full set here. It would make bumping it more automatic.

I'm not sure I understand: There is no plan/need to re-import these. Once the settings are synced across the repositories (already now) this is just for reference and possible future repositories.

Each repo would have a different set, but maybe we can merge it with another source, maybe even something fmf-y?

Yes, each repo has a completely different set of checks. I don't think it's worth automating in any way as we will use the import... let's say once per year when a need for a new repo pops up?

My concern here is that when we use an import, it would override it with the bogus value here.

It's not possible to override existing rule sets:

Name must be unique

So it's always needed to drop the rule set if it already exists.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should just add the full set here. It would make bumping it more automatic.

I'm not sure I understand: There is no plan/need to re-import these. Once the settings are synced across the repositories (already now) this is just for reference and possible future repositories.

What if a new security feature is introduced or we want to make any other changes.

Also having the test names can be useful every 6 months when we need to bump the fedora-stable names and drop the older ones.

@LecrisUT
LecrisUT requested a review from happz July 23, 2026 14:11
@psss
psss requested a review from LecrisUT July 29, 2026 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area | process The way how we collaborate on the development documentation Improvements or additions to documentation

Projects

Status: review

Development

Successfully merging this pull request may close these issues.

3 participants