Skip to content
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.

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

Display conflict-free merge messages for pull requests #15773

Conversation

justusbunsi
Copy link
Member

@justusbunsi justusbunsi commented May 7, 2021

External issue tracker often use a syntax like #1337 as item references in commits. When you handle pull requests with the default "merge commit" message, the very pull request will be referenced with # as well. Gitea itself can handle this situation and is able to identify both item and pull request reference when parsing the commit message.
If other external components (e.g.) push hooks ensure commits to only contain valid item references by looking for # occurrences, such commits would be rejected due to a non-valid issue reference.

Regarding the documentation, Gitea is able to use # and ! for item references for these use cases. This was handled only for squash merges

These changes take an external issue tracker into account when rendering merge commit messages. In such a case the documented ! will be used for item references in merge commits.

Signed-off-by: Steven Kriegler 61625851+justusbunsi@users.noreply.github.com

Repositories using external issue tracker tend to use numeric issues in
commits. To prevent conflicts during issue reference parsing or inside
commit hooks, this change respects these configuration and uses the !
character to refer to pull requests in merge commit messages.

For repositories using squash merges, this was already handled.

Signed-off-by: JustusBunsi <61625851+justusbunsi@users.noreply.github.com>
@6543 6543 added this to the 1.15.0 milestone May 7, 2021
@6543 6543 added the type/enhancement An improvement of existing functionality label May 7, 2021
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label May 8, 2021
models/pull.go Outdated Show resolved Hide resolved
models/pull.go Outdated Show resolved Hide resolved
models/pull.go Outdated Show resolved Hide resolved
@zeripath
Copy link
Contributor

zeripath commented May 8, 2021

@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf I suspect that this needs a second check to work for your situation?

@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor

@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf I suspect that this needs a second check to work for your situation?

Thanks for bringing me in!
Currently building with this patch.
How exactly would I be supposed to check, though? I should probably give it a proper look 🙈 Give me a minute, please.

Co-authored-by: zeripath <art27@cantab.net>
@justusbunsi justusbunsi requested a review from zeripath May 8, 2021 18:02
@zeripath
Copy link
Contributor

zeripath commented May 8, 2021

Actually @wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf you wouldn't use this - so I think you're fine. Sorry for wasting your time.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels May 8, 2021
@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor

Actually @wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf you wouldn't use this - so I think you're fine. Sorry for wasting your time.

yeah, no worries, now I see I'm not affected since I disable PRs for the mirrors and here it's if pr.BaseRepo.UnitEnabled(UnitTypeExternalTracker)..
This patch looks good, though.

Still, I have one interesting observation.
I briefly tried enabling PRs for the Gitea mirror I have, while the issue tracker is still set as shown here and while everything is still properly linking, I see no pulls tab.
It's not a result of this PR, so I suppose it must have been part of any of the recent changes.

@codecov-commenter
Copy link

Codecov Report

Merging #15773 (23c3c7a) into main (e22ee46) will decrease coverage by 0.01%.
The diff coverage is 62.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #15773      +/-   ##
==========================================
- Coverage   43.99%   43.97%   -0.02%     
==========================================
  Files         678      678              
  Lines       82016    82022       +6     
==========================================
- Hits        36081    36070      -11     
- Misses      40065    40077      +12     
- Partials     5870     5875       +5     
Impacted Files Coverage Δ
models/pull.go 55.13% <62.50%> (+0.80%) ⬆️
modules/util/timer.go 42.85% <0.00%> (-42.86%) ⬇️
services/pull/temp_repo.go 26.59% <0.00%> (-3.20%) ⬇️
services/pull/check.go 26.02% <0.00%> (-2.74%) ⬇️
modules/process/manager.go 72.83% <0.00%> (-2.47%) ⬇️
services/pull/patch.go 54.23% <0.00%> (-1.70%) ⬇️
modules/log/file.go 73.80% <0.00%> (-1.59%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e22ee46...23c3c7a. Read the comment docs.

@zeripath zeripath merged commit 864e656 into go-gitea:main May 8, 2021
@zeripath zeripath added the backport/done All backports for this PR have been created label May 8, 2021
zeripath added a commit to zeripath/gitea that referenced this pull request May 8, 2021
Backport go-gitea#15773

Repositories using external issue tracker tend to use numeric issues in
commits. To prevent conflicts during issue reference parsing or inside
commit hooks, this change respects these configuration and uses the !
character to refer to pull requests in merge commit messages.

For repositories using squash merges, this was already handled.

Signed-off-by: JustusBunsi <61625851+justusbunsi@users.noreply.github.com>
Co-authored-by: zeripath <art27@cantab.net>
@justusbunsi justusbunsi deleted the external-issue-references-in-merge-messages branch May 8, 2021 19:26
lunny pushed a commit that referenced this pull request May 9, 2021
Backport #15773

Repositories using external issue tracker tend to use numeric issues in
commits. To prevent conflicts during issue reference parsing or inside
commit hooks, this change respects these configuration and uses the !
character to refer to pull requests in merge commit messages.

For repositories using squash merges, this was already handled.

Signed-off-by: JustusBunsi <61625851+justusbunsi@users.noreply.github.com>
Co-authored-by: zeripath <art27@cantab.net>

Co-authored-by: Steven <61625851+justusbunsi@users.noreply.github.com>
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this pull request Aug 10, 2021
Repositories using external issue tracker tend to use numeric issues in
commits. To prevent conflicts during issue reference parsing or inside
commit hooks, this change respects these configuration and uses the !
character to refer to pull requests in merge commit messages.

For repositories using squash merges, this was already handled.

Signed-off-by: JustusBunsi <61625851+justusbunsi@users.noreply.github.com>
Co-authored-by: zeripath <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators Oct 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants