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

Rename actions unit to repo.actions and add docs for it #23733

Merged
merged 2 commits into from
Apr 3, 2023
Merged

Rename actions unit to repo.actions and add docs for it #23733

merged 2 commits into from
Apr 3, 2023

Conversation

wolfogre
Copy link
Member

@wolfogre wolfogre commented Mar 27, 2023

I neglected that the NameKey of Unit is not only for translation, but also configuration. So it should be repo.actions to maintain consistency.

⚠️ BREAKING ⚠️

If users already use actions.actions in DISABLED_REPO_UNITS or DEFAULT_REPO_UNITS, it will be treated as an invalid unit key.

@wolfogre wolfogre added type/bug pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! outdated/backport/v1.19 This PR should be backported to Gitea 1.19 labels Mar 27, 2023
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 27, 2023
@codecov-commenter
Copy link

codecov-commenter commented Mar 27, 2023

Codecov Report

Merging #23733 (e68ae8f) into main (f521e88) will decrease coverage by 0.02%.
The diff coverage is 42.00%.

❗ Current head e68ae8f differs from pull request most recent head 14b4430. Consider uploading reports for the commit 14b4430 to get more accurate results

@@            Coverage Diff             @@
##             main   #23733      +/-   ##
==========================================
- Coverage   47.14%   47.12%   -0.02%     
==========================================
  Files        1149     1155       +6     
  Lines      151446   152399     +953     
==========================================
+ Hits        71397    71824     +427     
- Misses      71611    72094     +483     
- Partials     8438     8481      +43     
Impacted Files Coverage Δ
cmd/dump.go 0.66% <0.00%> (-0.01%) ⬇️
cmd/web.go 0.00% <0.00%> (ø)
models/actions/run.go 1.64% <0.00%> (-0.08%) ⬇️
models/actions/runner.go 1.44% <ø> (ø)
models/packages/package.go 45.45% <0.00%> (-1.13%) ⬇️
models/unit/unit.go 50.79% <ø> (ø)
models/user/search.go 77.50% <0.00%> (-6.29%) ⬇️
modules/actions/workflows.go 0.00% <0.00%> (ø)
modules/context/context.go 64.54% <0.00%> (-3.53%) ⬇️
modules/doctor/storage.go 30.65% <0.00%> (-1.29%) ⬇️
... and 39 more

... and 44 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@delvh delvh left a comment

Choose a reason for hiding this comment

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

The good thing is:
Since it was previously undocumented, there likely aren't a lot of affected users.
I mean, I myself didn't know that this was possible, and I reviewed the original PR and have gotten somewhat familiar with the code base 😅

models/unit/unit.go Show resolved Hide resolved
@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 27, 2023
@delvh delvh added this to the 1.20.0 milestone Mar 27, 2023
@delvh delvh added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 27, 2023
@yardenshoham
Copy link
Member

Backporting a breaking PR?

@wolfogre
Copy link
Member Author

Backporting a breaking PR?

I hope it can also be fixed on v1.19.1.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Mar 27, 2023

We have the backport-locales.go, so I guess backporting some translations are feasible now.

About the "name" breaking, I guess it won't cause serious problems, so I would assume it's fine?

Copy link
Member

@techknowlogick techknowlogick left a comment

Choose a reason for hiding this comment

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

Blocking, as we shouldn't be backporting breaking changes. Is there a way to make the previous option an alias for the update option?

@lunny
Copy link
Member

lunny commented Mar 27, 2023

Blocking, as we shouldn't be backporting breaking changes. Is there a way to make the previous option an alias for the update option?

Since we have said actions is an experiment feature in v1.19, I think we could change it directly.

BTW: maybe we should also update repo_unit table? @wolfogre

@yp05327
Copy link
Contributor

yp05327 commented Mar 28, 2023

Just a question. Is it necessary/possible to change action_XXX to repo_action_XXX in db too, as we also have activities.action which has action table in db.

@lunny
Copy link
Member

lunny commented Mar 28, 2023

It should be another PR. Maybe we can rename the old table action to another name.

@wolfogre
Copy link
Member Author

wolfogre commented Mar 28, 2023

BTW: maybe we should also update repo_unit table?

@lunny

Update what?

type RepoUnit struct { //revive:disable-line:exported
	ID          int64
	RepoID      int64              `xorm:"INDEX(s)"`
	Type        unit.Type          `xorm:"INDEX(s)"`
	Config      convert.Conversion `xorm:"TEXT"`
	CreatedUnix timeutil.TimeStamp `xorm:"INDEX CREATED"`
}

It doesn't use the key string, just unit.Type. And if there's an existing repo with Actions enabled, we shouldn't touch it.

@wxiaoguang
Copy link
Contributor

By the way, there is another strong reason about why this "breaking" could be backported: Gitea declares that the Actions feature is experimental, so it's not surprised that the stable release needs some "breaking" fixes for such experimental feature.

@lunny
Copy link
Member

lunny commented Mar 29, 2023

@techknowlogick please review again. I think the break is just an option on the ini file from actions.actions to repo.actions. @wolfogre

@lunny lunny removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 29, 2023
@techknowlogick techknowlogick dismissed their stale review April 3, 2023 03:25

dismiss my review

@techknowlogick techknowlogick enabled auto-merge (squash) April 3, 2023 03:25
@6543 6543 added backport/manual No power to the bots! Create your backport yourself! and removed backport/manual No power to the bots! Create your backport yourself! labels Apr 3, 2023
@6543
Copy link
Member

6543 commented Apr 3, 2023

also ok with backpor that ...

@techknowlogick techknowlogick merged commit 977ef21 into go-gitea:main Apr 3, 2023
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Apr 3, 2023
…3733)

I neglected that the `NameKey` of `Unit` is not only for translation,
but also configuration. So it should be `repo.actions` to maintain
consistency.

## ⚠️ BREAKING ⚠️

If users already use `actions.actions` in `DISABLED_REPO_UNITS` or
`DEFAULT_REPO_UNITS`, it will be treated as an invalid unit key.
@GiteaBot GiteaBot added the backport/done All backports for this PR have been created label Apr 3, 2023
6543 pushed a commit that referenced this pull request Apr 3, 2023
…23881)

Backport #23733 by @wolfogre

I neglected that the `NameKey` of `Unit` is not only for translation,
but also configuration. So it should be `repo.actions` to maintain
consistency.

## ⚠️ BREAKING ⚠️

If users already use `actions.actions` in `DISABLED_REPO_UNITS` or
`DEFAULT_REPO_UNITS`, it will be treated as an invalid unit key.

Co-authored-by: Jason Song <i@wolfogre.com>
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 4, 2023
* upstream/main:
  Use User.ID instead of User.Name in ActivityPub API for Person IRI (go-gitea#23823)
  Remove fomantic ".link" selector and styles (go-gitea#23888)
  [skip ci] Updated translations via Crowdin
  Fix `cases.Title` crash for concurrency (go-gitea#23885)
  Disable editing tags (go-gitea#23883)
  Fix user profile description rendering (go-gitea#23882)
  Introduce GiteaLocaleNumber custom element to handle number localization on pages. (go-gitea#23861)
  Convert .Source.SkipVerify to $cfg.SkipVerify (go-gitea#23839)
  Fix review box viewport overflow issue (go-gitea#23800)
  Fix owner team access mode value in team_unit table (go-gitea#23675)
  Fix submit button won't refresh in New Repository Fork page (go-gitea#22994)
  Introduce GitHub markdown editor, keep EasyMDE as fallback (go-gitea#23876)
  Improve LoadUnitConfig to handle invalid or duplicate units (go-gitea#23736)
  Append `(comment)` when a link points at a comment rather than the whole issue (go-gitea#23734)
  Rename actions unit to `repo.actions` and add docs for it (go-gitea#23733)
  Try to catch more broken translations (go-gitea#23867)
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
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. outdated/backport/v1.19 This PR should be backported to Gitea 1.19 pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants