Skip to content

Add support for allowing edits from maintainers on PRs/MRs - #1011

Open
betulependule wants to merge 2 commits into
packit:mainfrom
betulependule:forges/allow-maintainer-edit
Open

Add support for allowing edits from maintainers on PRs/MRs#1011
betulependule wants to merge 2 commits into
packit:mainfrom
betulependule:forges/allow-maintainer-edit

Conversation

@betulependule

@betulependule betulependule commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

PR summary

This PR adds support for enabling edits from maintainers on PRs/MRs across all forges, which support this (GitHub, GitLab and Forgejo). The Forgejo API doesn't allow setting this directly when creating a PR, so in this case, it is only supported when editing the PR created. Pagure doesn't support this option.

This functionality is needed in preparation for the migration of dist-git to Forgejo, but it was implemented for the other forges as well for consistency's sake.

While working on this, I also improved the implementation of PullRequest.update_info() for GitHub, which seemed problematic as it could raise an error. The fix is related to this code and is small enough that I don't think it would deserve a separate PR, so I included it as a separate commit.

Tests

Integration tests have been added for GitHub and GitLab.

Our Forgejo testing repo is hosted on v10.next.forgejo.org, which is past its EOL, so I was not able to create an account there to be able to run and record an integration test for Forgejo. I would generally use our packit-validator account for creating tests, but for this particular test, I would need a different account in order to create a fork and a PR from it.

As for Pagure, the tests are hosted on pagure.io, which is currently read-only, so I was not able to run a test for it, either. Considering that Pagure doesn't support enabling edits from maintainers anyway, this isn't a big deal.

Related to: #2748

RELEASE NOTES BEGIN

In ogr, a potential bug has been fixed in the GitHub implementation of PullRequest.update_info(). Previously, None values could be passed to the underlying method called to edit a PR, which could result in an AssertionError being raised as this method doesn't support None values.

ogr now supports the allow_maintainer_edit option for PRs/MRs for GitHub, GitLab and Forgejo. As the Forgejo API doesn't allow to set this directly when creating a PR, allow_maintainer_edit can only be set when updating it.

RELEASE NOTES END

The `title` and `description` parameter can be `None` and were passed
directly to the `github` module's edit() method, which does not
accept `None` (instead it expects the `NotSet` type). This would result
in an error being raised. The method has been fixed to prevent this
from occurring.
`ogr` now supports the `allow_maintainer_edit` attribute on PR/MR
objects for GitLab, GitHub and Forgejo.

It is now possible to specify `allow_maintainer_edit` when creating
a PR/MR on GitHub and GitLab using `ogr`. The Forgejo API doesn't
accept this option directly when creating a PR, so attempting to
set it raises an `OperationNotSupported` error.

All three forges (GitHub, GitLab and Forgejo) support setting
`allow_maintainer_edit` when editting a PR/MR.

The Pagure API doesn't support allowing edits from maintainers, so the
implementation of `allow_maintainer_edit` was omitted for Pagure.
@betulependule
betulependule requested a review from a team as a code owner August 26, 2026 15:19
@betulependule
betulependule requested review from lbarcziova and removed request for a team August 26, 2026 15:19
@centosinfra-prod-github-app

Copy link
Copy Markdown
Contributor

@lbarcziova lbarcziova 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.

LGTM, just one note

allow_maintainer_edit=True,
)

pr = self.project.get_pr(pr.id)

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.

shouldnt this be called on project. instead of self.project. ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

project in this case refers to the fork project while self.project references the original repo. pr.id identifies the PR within the original repo, so using project wouldn't work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants