Skip to content

Document what is required before merging a pull request #5065

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
psss wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Document what is required before merging a pull request #5065

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1edfc8a
Document what is required before merging a pull request
psss Jul 17, 2026
22fce61
Enforce 2 reviews, drop source/id, add check placeholder
psss Jul 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions .github/settings/enforced.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
{
"id": 17224362,
"name": "Merging into main (rules enforced for everybody)",
"target": "branch",
"source_type": "Repository",
"source": "teemtee/tmt",
Comment thread
psss marked this conversation as resolved.
Outdated
"enforcement": "active",
"conditions": {
"ref_name": {
"exclude": [],
"include": [
"~DEFAULT_BRANCH"
]
}
},
"rules": [
{
"type": "non_fast_forward"
},
{
"type": "required_linear_history"
},
{
"type": "pull_request",
"parameters": {
"required_approving_review_count": 0,
"dismiss_stale_reviews_on_push": false,
"required_reviewers": [],
"require_code_owner_review": false,
"dismissal_restriction": {
"enabled": false,
"allowed_actors": []
},
"require_last_push_approval": false,
"required_review_thread_resolution": true,
"allowed_merge_methods": [
"squash"
]
}
},
{
"type": "deletion"
}
],
"bypass_actors": []
}
43 changes: 43 additions & 0 deletions .github/settings/override.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
{
"id": 17224740,
"name": "Merging into main (release lead can override)",
"target": "branch",
"source_type": "Repository",
"source": "teemtee/tmt",
"enforcement": "active",
"conditions": {
"ref_name": {
"exclude": [],
"include": [
"~DEFAULT_BRANCH"
]
}
},
"rules": [
{
"type": "pull_request",
"parameters": {
"required_approving_review_count": 2,
"dismiss_stale_reviews_on_push": false,
"required_reviewers": [],
"require_code_owner_review": false,
"dismissal_restriction": {
"enabled": false,
"allowed_actors": []
},
"require_last_push_approval": false,
"required_review_thread_resolution": false,
"allowed_merge_methods": [
"squash"
]
Comment on lines +20 to +24

@LecrisUT LecrisUT Jul 23, 2026

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.

@psss psss Jul 29, 2026

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.

@LecrisUT LecrisUT Jul 29, 2026

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.

}
}
],
"bypass_actors": [
{
"actor_id": 17396698,
"actor_type": "Team",
"bypass_mode": "pull_request"
}
]
}
21 changes: 16 additions & 5 deletions docs/contribute.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -925,14 +925,25 @@ Chat
Merging
------------------------------------------------------------------

Pull request merging is done by one of maintainers who have a good
overview of the whole code. Maintainer who will take care of
the process will assign themselves to the pull request.
Before merging it's good to check the following:
Pull request merging is done by maintainers who have the write
permission to the ``main`` branch. The following is required
before merging a pull request:

* At least two positive reviews provided
* All review conversations have been resolved

Before merging it's also good to check the following:

* New test coverage added if appropriate, all tests passed
* Documentation has been added or updated where appropriate
* At least two positive reviews provided by the maintainers

Some settings, like conversation resolution, are `enforced`__ by
the ruleset. The release lead can `override`__ some requirements
if needed, for example documentation changes do not need the full
test coverage to be exercised. Use the override responsibly.

__ https://github.com/teemtee/tmt/blob/main/.github/settings/enforced.json
__ https://github.com/teemtee/tmt/blob/main/.github/settings/override.json

Merge commits are not used, rebase on the latest ``main`` instead.
Use the GitHub's ``Squash & Merge`` button which will generate the
Expand Down
Loading