Skip to content

Conversation

6543
Copy link
Member

@6543 6543 commented Mar 30, 2022

part of #9307

This make checks in one single place so they dont differ and maintainer can not forget a check in one place while adding it to the other .... ( as it's atm )

Fix:

  • The API does ignore issue dependencies where Web does not
  • The API checks if "IsSignedIfRequired" where Web does not - UI probably do but nothing will some to craft custom requests
  • Default merge message is crafted a bit different between API and Web if not set on specific cases ...

@6543 6543 added type/bug type/refactoring Existing code has been cleaned up. There should be no new functionality. labels Mar 30, 2022
@6543 6543 added this to the 1.17.0 milestone Mar 30, 2022
@6543 6543 requested a review from wxiaoguang March 31, 2022 02:06
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 31, 2022
@wxiaoguang
Copy link
Contributor

It's marked as bug, what bug does it fix? Otherwise it looks good

@6543
Copy link
Member Author

6543 commented Mar 31, 2022

@wxiaoguang

  • the API does ignore IssueDependencies where Web does not
  • the API checks if "IsSignedIfRequired" where Web does not - ui probably do but nothing will some to craft custom requests
  • Default merge message is craffted a bit different between API and Web if not set on specific cases ...

all hapens doue to copy code instead of having a single function for the job ...
... and with #9307 it would be three - so i took the chance to make it right

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Mar 31, 2022

@wxiaoguang

* the API does ignore IssueDependencies where Web does not

* the API checks if "IsSignedIfRequired" where Web does not - ui probably do but nothing will some to craft custom requests

* Default merge message is craffted a bit different between API and Web if not set on specific cases ...

all hapens doue to copy code instead of having a single function for the job

Wow, that's a lot, I think it's worth to mention them in the commit message (when merging) 😊

@6543
Copy link
Member Author

6543 commented Mar 31, 2022

pull desc updated

Copy link
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.

LGTM (some non-blocker small nits)

@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 Mar 31, 2022
@6543
Copy link
Member Author

6543 commented Mar 31, 2022

@wxiaoguang added some proposed unrelated refactoring ...

@6543 6543 requested a review from lunny March 31, 2022 13:29
@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 Mar 31, 2022
@6543 6543 changed the title Move checks for branchprotection into own package Move checks for pulls before merge into own function Mar 31, 2022
@6543 6543 merged commit 9c349a4 into go-gitea:main Mar 31, 2022
@6543 6543 deleted the single-place-to-enforce-protected-branch-rules branch March 31, 2022 14:53
6543 added a commit that referenced this pull request Mar 31, 2022
Backport #19271

Fix:
* The API does ignore issue dependencies where Web does not
* The API checks if "IsSignedIfRequired" where Web does not - UI probably do but nothing will some to craft custom requests
* Default merge message is crafted a bit different between API and Web if not set on specific cases ...
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 1, 2022
* giteaoffical/main:
  Fix broken of team create (go-gitea#19288)
  Remove `git.Command.Run` and `git.Command.RunInDir*` (go-gitea#19280)
  Performance improvement for add team user when org has more than 1000 repositories (go-gitea#19227)
  [skip ci] Updated translations via Crowdin
  Update JS dependencies (go-gitea#19281)
  Fix container download counter (go-gitea#19287)
  go.mod: update kevinburke/ssh_config to v1.2.0 (go-gitea#19286)
  Fix global packages enabled avaiable (go-gitea#19276)
  Add Goroutine stack inspector to admin/monitor (go-gitea#19207)
  Move checks for pulls before merge into own function (go-gitea#19271)
  Restore user autoregistration with email addresses (go-gitea#19261)
  Improve sync performance for pull-mirrors (go-gitea#19125)
  Refactor `git.Command.Run*`, introduce `RunWithContextString` and `RunWithContextBytes` (go-gitea#19266)
  Move reaction to models/issues/ (go-gitea#19264)
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants