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.

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

Jump to bottom

Also run automerge check after pull review #25741

Closed
wants to merge 8 commits into from
Closed

Also run automerge check after pull review #25741

wants to merge 8 commits into from
+186 −78

Conversation

6543
Copy link
Member

@6543 6543 commented Jul 7, 2023 •
edited by lunny
Loading

Close #24445
Fix #30658

@6543 6543 added the type/enhancement An improvement of existing functionality label Jul 7, 2023
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 7, 2023
@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 7, 2023
@6543 6543 added this to the 1.22.0 milestone Sep 2, 2023
@rombert
Copy link

rombert commented Dec 20, 2023

Is this related to #24445 ?

@6543
Copy link
Member Author

6543 commented Dec 21, 2023

most likely - I'll have to create a serous pull, this was more a reminder to let me finish it

@lunny
Copy link
Member

lunny commented Dec 21, 2023

Automerge feature needs a redesign like Github did.

@pull-request-size pull-request-size bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 2, 2024
@github-actions github-actions bot added the modifies/api This PR adds API routes or modifies them label Mar 2, 2024
@6543 6543 marked this pull request as ready for review March 2, 2024 07:36
@6543 6543 changed the title WIP: also run automerge after pull review Also run automerge check after pull review Mar 2, 2024
@6543 6543 requested a review from a team March 2, 2024 07:36
@6543 6543 added type/bug and removed type/enhancement An improvement of existing functionality labels Mar 2, 2024
@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 4, 2024
@6543
Copy link
Member Author

6543 commented Mar 4, 2024 •
edited
Loading

hmm it affects the integration tests ... I need to look if we depend on the broken behavior in our tests or if tests are not isolated enough

@lunny
Copy link
Member

lunny commented Apr 9, 2024

file conflicted.

@github-actions github-actions bot added modifies/translation modifies/go Pull requests that update Go code modifies/cli PR changes something on the CLI, i.e. gitea doctor or gitea admin modifies/templates This PR modifies the template files modifies/docs modifies/migrations modifies/internal modifies/js modifies/dependencies labels Apr 18, 2024
@lunny lunny force-pushed the also_auto-merge_on_review_updates branch from 7445aae to 670e236 Compare April 18, 2024 09:29
@lunny lunny force-pushed the also_auto-merge_on_review_updates branch from 7435a03 to db4113e Compare April 24, 2024 09:03
@pull-request-size pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Apr 24, 2024
@yp05327
Copy link
Contributor

yp05327 commented Apr 24, 2024 •
edited
Loading

@lunny
Another big problem, Gitea server will freeze for a long time after the following steps:
image

reproduce:
shut down the runner
user B: create a new branch and PR, and enable auto merge
user A: approve it, then we will wait for the CI
start your runner, let the CI finish, then it should be merged as expected, but no, Gitea server will freeze for a long time.

After the frozen, you will see this:
image

Not sure what happened, I have no time to look into it.

@lunny lunny modified the milestones: 1.22.0, 1.23.0 Apr 28, 2024
This was referenced Apr 30, 2024
Merged
Merged
lunny added a commit that referenced this pull request May 7, 2024
@lunny @wxiaoguang
Move database operations of merging a pull request to post receive ho…
ebf0c96 
…ok and add a transaction (#30805)

Merging PR may fail because of various problems. The pull request may
have a dirty state because there is no transaction when merging a pull
request. ref
#25741 (comment)

This PR moves all database update operations to post-receive handler for
merging a pull request and having a database transaction. That means if
database operations fail, then the git merging will fail, the git client
will get a fail result.

There are already many tests for pull request merging, so we don't need
to add a new one.

---------

Co-authored-by: wxiaoguang <[email protected]>
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request May 7, 2024
@lunny @wxiaoguang @GiteaBot
Move database operations of merging a pull request to post receive ho…
e5c7494 
…ok and add a transaction (go-gitea#30805)

Merging PR may fail because of various problems. The pull request may
have a dirty state because there is no transaction when merging a pull
request. ref
go-gitea#25741 (comment)

This PR moves all database update operations to post-receive handler for
merging a pull request and having a database transaction. That means if
database operations fail, then the git merging will fail, the git client
will get a fail result.

There are already many tests for pull request merging, so we don't need
to add a new one.

---------

Co-authored-by: wxiaoguang <[email protected]>
@GiteaBot GiteaBot mentioned this pull request May 7, 2024
Merged
@lunny
Copy link
Member

lunny commented May 7, 2024

replaced by #30780

@lunny lunny closed this May 7, 2024
@GiteaBot GiteaBot removed this from the 1.23.0 milestone May 7, 2024
lunny added a commit that referenced this pull request May 8, 2024
@GiteaBot @lunny @wxiaoguang
Move database operations of merging a pull request to post receive ho…
ec3f5f9 
…ok and add a transaction (#30805) (#30888)

Backport #30805 by @lunny

Merging PR may fail because of various problems. The pull request may
have a dirty state because there is no transaction when merging a pull
request. ref
#25741 (comment)

This PR moves all database update operations to post-receive handler for
merging a pull request and having a database transaction. That means if
database operations fail, then the git merging will fail, the git client
will get a fail result.

There are already many tests for pull request merging, so we don't need
to add a new one.

Co-authored-by: Lunny Xiao <[email protected]>
Co-authored-by: wxiaoguang <[email protected]>
DennisRasey pushed a commit to DennisRasey/forgejo that referenced this pull request May 14, 2024
@lunny @earl-warren
Move database operations of merging a pull request to post receive ho…
eb792d9 
…ok and add a transaction (#30805)

Merging PR may fail because of various problems. The pull request may
have a dirty state because there is no transaction when merging a pull
request. ref
go-gitea/gitea#25741 (comment)

This PR moves all database update operations to post-receive handler for
merging a pull request and having a database transaction. That means if
database operations fail, then the git merging will fail, the git client
will get a fail result.

There are already many tests for pull request merging, so we don't need
to add a new one.

---------

Co-authored-by: wxiaoguang <[email protected]>
(cherry picked from commit ebf0c969403d91ed80745ff5bd7dfbdb08174fc7)

Conflicts:
	modules/private/hook.go
	routers/private/hook_post_receive.go
	trivial conflicts because
	  263a716cb5 * Performance optimization for git push (#30104)
	was not cherry-picked and because of
	  998a431 Do not update PRs based on events that happened before they existed
DennisRasey pushed a commit to DennisRasey/forgejo that referenced this pull request May 16, 2024
@lunny @earl-warren
Move database operations of merging a pull request to post receive ho…
7e81775 
…ok and add a transaction (#30805)

Merging PR may fail because of various problems. The pull request may
have a dirty state because there is no transaction when merging a pull
request. ref
go-gitea/gitea#25741 (comment)

This PR moves all database update operations to post-receive handler for
merging a pull request and having a database transaction. That means if
database operations fail, then the git merging will fail, the git client
will get a fail result.

There are already many tests for pull request merging, so we don't need
to add a new one.

---------

Co-authored-by: wxiaoguang <[email protected]>
(cherry picked from commit ebf0c969403d91ed80745ff5bd7dfbdb08174fc7)

Conflicts:
	modules/private/hook.go
	routers/private/hook_post_receive.go
	trivial conflicts because
	  263a716cb5 * Performance optimization for git push (#30104)
	was not cherry-picked and because of
	  998a431 Do not update PRs based on events that happened before they existed
(cherry picked from commit eb792d9)

(cherry picked from commit ec3f5f9992d7ff8250c044a4467524d53bd50210)
lunny added a commit that referenced this pull request May 21, 2024
@lunny @6543 @wxiaoguang
Fix automerge will not work because of some events haven't been trigg…
c6cf96d 
…ered (#30780)

Replace #25741
Close #24445
Close #30658
Close #20646
~Depends on #30805~

Since #25741 has been rewritten totally, to make the contribution
easier, I will continue the work in this PR. Thanks @6543

---------

Co-authored-by: 6543 <[email protected]>
Co-authored-by: wxiaoguang <[email protected]>
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request May 21, 2024
@lunny @6543
@wxiaoguang @GiteaBot
Fix automerge will not work because of some events haven't been trigg…
d3d5ace 
…ered (go-gitea#30780)

Replace go-gitea#25741
Close go-gitea#24445
Close go-gitea#30658
Close go-gitea#20646
~Depends on go-gitea#30805~

Since go-gitea#25741 has been rewritten totally, to make the contribution
easier, I will continue the work in this PR. Thanks @6543

---------

Co-authored-by: 6543 <[email protected]>
Co-authored-by: wxiaoguang <[email protected]>
@GiteaBot GiteaBot mentioned this pull request May 21, 2024
Merged
lunny added a commit that referenced this pull request May 22, 2024
@GiteaBot @lunny
@6543 @wxiaoguang
Fix automerge will not work because of some events haven't been trigg…
2648962 
…ered (#30780) (#31039)

Backport #30780 by @lunny

Replace #25741
Close #24445
Close #30658
Close #20646
~Depends on #30805~

Since #25741 has been rewritten totally, to make the contribution
easier, I will continue the work in this PR. Thanks @6543

Co-authored-by: Lunny Xiao <[email protected]>
Co-authored-by: 6543 <[email protected]>
Co-authored-by: wxiaoguang <[email protected]>
@wxiaoguang wxiaoguang removed the backport/v1.22 This PR should be backported to Gitea 1.22 label Jun 20, 2024
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Aug 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lunny lunny lunny left review comments

@yp05327 yp05327 yp05327 left review comments

@wxiaoguang wxiaoguang wxiaoguang approved these changes

Assignees
No one assigned
Labels
backport/v1.21 This PR should be backported to Gitea 1.21 lgtm/need 1 This PR needs approval from one additional maintainer to be merged. modifies/go Pull requests that update Go code size/L Denotes a PR that changes 100-499 lines, ignoring generated files. type/bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Can not create merge commit when checks succeed Automerge does not work
6 participants
@6543 @rombert @lunny @yp05327 @wxiaoguang @GiteaBot