Skip to content

Change default "allow edits from maintainers" to true#36709

Closed
silverwind wants to merge 3 commits intogo-gitea:mainfrom
silverwind:allow-edit
Closed

Change default "allow edits from maintainers" to true#36709
silverwind wants to merge 3 commits intogo-gitea:mainfrom
silverwind:allow-edit

Conversation

@silverwind
Copy link
Copy Markdown
Member

@silverwind silverwind commented Feb 22, 2026

This aligns with GitHub's default behavior where the "Allow edits from maintainers" checkbox is checked by default when creating pull requests.

No migration needed or desired, this will apply to new pull requests only.

This aligns with GitHub's default behavior where the "Allow edits from
maintainers" checkbox is checked by default when creating pull requests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 22, 2026
@github-actions github-actions bot added modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code labels Feb 22, 2026
@silverwind silverwind requested a review from Copilot February 22, 2026 16:38
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Feb 22, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aligns Gitea's default behavior with GitHub by changing the default value of "Allow edits from maintainers" to true when creating pull requests. This means that by default, maintainers of the base repository will be able to edit pull requests from forks, making collaboration easier.

Changes:

  • Updated database schema default for AllowMaintainerEdit field from false to true
  • Updated all code locations that set or use this default value to ensure consistency

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
models/issues/pull.go Changed database schema default from DEFAULT false to DEFAULT true for the AllowMaintainerEdit column
services/convert/repository.go Updated default value initialization from false to true when converting repository data for API responses
routers/web/repo/compare.go Changed fallback default from false to true when repository unit config is unavailable
routers/api/v1/repo/repo.go Updated default configuration value from false to true when creating new pull request unit settings via API

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Feb 22, 2026

Actually this isn't as straightforward as it seemed because the default of that flag is stored in the repo, not in the pull request, investigating...

Updates all existing repo pull request configs to set
DefaultAllowMaintainerEdit to true, matching the new default.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Feb 22, 2026

The bool field has no json:"omitempty" tag, so false is explicitly stored in the DB for all repos. Unfortunately this means the only clean fix is a one-time migration from false to true.

I still think it's the right thing to do. Gitea is about collaboration and this flag just has to default to true, I use this functionality daily to edit PRs.

@silverwind silverwind requested a review from lafriks February 22, 2026 16:59
@GiteaBot GiteaBot added lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Feb 22, 2026
@silverwind
Copy link
Copy Markdown
Member Author

@lafriks check if you're okay with the migration.

Copy link
Copy Markdown
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

You are breaking the instances whose repo owners have explicitly disabled "DefaultAllowMaintainerEdit"

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Feb 22, 2026
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Feb 22, 2026

What exactly is "breaking"? How would you suggest we handle this? Make it apply only to new repos?

I can migrate my repos on my instance myself after merging this, just thought I'd be nice and apply it to all repos.

@wxiaoguang
Copy link
Copy Markdown
Contributor

Make it apply only to new repos?

Why not? The first rule is don't change user's setting without notifying them and/or it is not a must. Right?

Have you seen GitHub changes your settings?

@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Feb 22, 2026

The problem is we can't know whether a admin has consciously changed this or not (because of missing omitempty). Most probably haven't and I can assure no one will go through repos one-by-one fixing this.

What harm would this setting do? Maintainers already have push permission to repos, so they can edit the branch after merge anyways. It's just a inconvenience not being able to edit.

@wxiaoguang
Copy link
Copy Markdown
Contributor

Don't you see quite a lot of people prefer strict permissions? For example: Option to turn off ability for administrator to merge pull request without getting approvals granted #17131. Take a look at the "upvotes" there.

@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Feb 22, 2026

Ok fine then I will revert the migration. I can manually do the migration for my instance and I hope someone does it for gitea.com because it annoys me there as well.

BaseBranch string
MergeBase string `xorm:"VARCHAR(64)"`
AllowMaintainerEdit bool `xorm:"NOT NULL DEFAULT false"`
AllowMaintainerEdit bool `xorm:"NOT NULL DEFAULT true"`
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.

I believe it affects nothing. Need to revert

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.

If it affects nothing than default should be removed at all, keeping it different for other settings is worse

@wxiaoguang
Copy link
Copy Markdown
Contributor

Still not right. I don't see where DefaultAllowMaintainerEdit is really set for the PR unit.

@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Feb 22, 2026

BTW I think we should also implement this green checkmark in the sidebar to indicate this flag to everyone, can probably do it in this PR.

https://github.blog/changelog/2023-02-10-see-when-a-pull-request-is-editable-by-maintainers/

@wxiaoguang
Copy link
Copy Markdown
Contributor

BTW I think we should also implement this green checkmark in the sidebar to indicate this flag to everyone, can probably do it in this PR.

https://github.blog/changelog/2023-02-10-see-when-a-pull-request-is-editable-by-maintainers/

Haven't I done that? You also reviewed and approved.

Show info about maintainers are allowed to edit a PR (#33738)

@ChristopherHX
Copy link
Copy Markdown
Contributor

Haven't I done that? You also reviewed and approved.

Yes this exist and I have seen it on gitea.com

@silverwind
Copy link
Copy Markdown
Member Author

Yeah it exists, I guess it just needs some minor touch up on the styling (green checkmark and regular text color). I rarely deal with forked repos so I never noticed.

image

@wxiaoguang
Copy link
Copy Markdown
Contributor

Yeah it exists,

I have said dozens of times that "please don't just guess or imagine" to you in recent days, especially after you started using AI.

@wxiaoguang
Copy link
Copy Markdown
Contributor

"DefaultAllowMaintainerEdit=true" is in "Fix API not persisting pull request unit config when has_pull_requests is not set #36718"

@wxiaoguang wxiaoguang closed this Mar 2, 2026
@silverwind silverwind deleted the allow-edit branch March 26, 2026 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants