Skip to content

Cleanup ActionRun creation #35624

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
Merged
merged 16 commits into from
Oct 12, 2025
Merged

Cleanup ActionRun creation #35624

merged 16 commits into from
Oct 12, 2025
+133 −236

Conversation

ChristopherHX
Copy link
Contributor

@ChristopherHX ChristopherHX commented Oct 10, 2025 •
edited
Loading

  • share more logic

Closes #35622
@ChristopherHX
Cleanup ActionRun creation
678a970 
* share more logic
@ChristopherHX ChristopherHX added type/bug type/refactoring Existing code has been cleaned up. There should be no new functionality. topic/gitea-actions related to the actions of Gitea labels Oct 10, 2025
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 10, 2025
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Oct 10, 2025
@ChristopherHX ChristopherHX marked this pull request as ready for review October 11, 2025 10:51
@ChristopherHX
Copy link
Contributor Author

This shows how, worse the duplication is at the moment. Preparation for further refactoring.

Maybe

  • merge the new PrepareRun with InsertRun
  • change the name of PrepareRun

At least the surface of doing code path specific inconsistent changes decreases and more tests cover the shared code than the specific code ever did.
ChristopherHX
ChristopherHX commented Oct 11, 2025
View reviewed changes
delvh
delvh approved these changes Oct 11, 2025
View reviewed changes
@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 Oct 11, 2025
Zettat123
Zettat123 reviewed Oct 11, 2025
View reviewed changes
ChristopherHX and others added 3 commits October 11, 2025 20:49
@ChristopherHX @delvh
Update services/actions/run.go
948f650 
Co-authored-by: delvh <dev.lh@web.de> Signed-off-by: ChristopherHX <christopher.homberger@web.de>
@ChristopherHX
rename func
0cc6d31
@ChristopherHX
do not query vars twice
a5aea1b
@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 Oct 11, 2025
wxiaoguang
wxiaoguang reviewed Oct 12, 2025
View reviewed changes
@github-actions github-actions bot added the modifies/api This PR adds API routes or modifies them label Oct 12, 2025
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Oct 12, 2025 •
edited
Loading

Made some new changes, now we require callers to pass "run" when calling CreateCommitStatusForRunJobs, then it is CreateCommitStatusForRunJobs's responsibility to decide whether to create the status (for example: no status for cron/scheduled jobs). And we only need to retrieve event name and commit id once for a run.
@ChristopherHX
Copy link
Contributor Author

Thank you for your additional refactoring. I had something similar in my mind for optimizing the code later.
@wxiaoguang wxiaoguang enabled auto-merge (squash) October 12, 2025 12:22
@wxiaoguang wxiaoguang merged commit 2401812 into go-gitea:main Oct 12, 2025
26 checks passed
@GiteaBot GiteaBot added this to the 1.26.0 milestone Oct 12, 2025
zjjhot added a commit to zjjhot/gitea that referenced this pull request Oct 13, 2025
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
d15f230 
* giteaofficial/main: [skip ci] Updated translations via Crowdin Update JS deps, misc tweaks (go-gitea#35643) Bump actions/checkout to v5 (go-gitea#35644) nix flake update (go-gitea#35639) Cleanup ActionRun creation (go-gitea#35624) bump archives&rar dep (go-gitea#35637) Fix merge panic (go-gitea#35606) Bump happy-dom from 19.0.2 to 20.0.0 (go-gitea#35625) Refactor ActionRunJob parsing into a reusable function (go-gitea#35623) Fix code tag style problem and LFS view bug (go-gitea#35628) Support Actions `concurrency` syntax (go-gitea#32751) The status icon of the Action step is consistent with GitHub (go-gitea#35618) Mock external service in hcaptcha TestCaptcha (go-gitea#35604) Fix inputing review comment will remove reviewer (go-gitea#35591) [skip ci] Updated translations via Crowdin Fix diffpatch API endpoint (go-gitea#35610) Print PR-Title into tooltip for actions (go-gitea#35579)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code topic/gitea-actions related to the actions of Gitea type/bug type/refactoring Existing code has been cleaned up. There should be no new functionality.

5 participants

@ChristopherHX @wxiaoguang @Zettat123 @delvh @GiteaBot