Skip to content

Disable all mirror feature elements when DISABLE_MIRRORS is enabled #13084

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

Closed
wants to merge 1 commit into from

Conversation

pboguslawski
Copy link
Contributor

Parameter DISABLE_MIRRORS should disable all elements of
mirroring feature (i.e. filters in UI, updating cron job)
not just creating new mirrors. This mod fixes it and still
allows exiting mirrors to be converted to regular repos when
DISABLE_MIRRORS=true.

If one need disabling only new mirror creation, should
create mod with separate parameter like DISABLE_MIRROR_CREATE.

Fixes: a6fd2f2
Author-Change-Id: IB#1105104

Parameter DISABLE_MIRRORS should disable all elements of
mirroring feature (i.e. filters in UI, updating cron job)
not just creating new mirrors. This mod fixes it and still
allows exiting mirrors to be converted to regular repos when
DISABLE_MIRRORS=true.

If one need disabling only new mirror creation, should
create mod with separate parameter like DISABLE_MIRROR_CREATE.

Fixes: a6fd2f2
Author-Change-Id: IB#1105104
@lafriks lafriks added the pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! label Oct 9, 2020
@6543
Copy link
Member

6543 commented Oct 11, 2020

I was thining about DISABLE_MIRRORS=true|create|false but this would complicate settings code ... so not a good idear

@pboguslawski this pull remove "disable mirror creation" and add a new feature ... can you add DISABLE_MIRROR_CREATE too so this pull will only add a feature :)

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 11, 2020
@6543 6543 added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Oct 11, 2020
@pboguslawski
Copy link
Contributor Author

DISABLE_MIRRORS should do what it means - disable whole mirroring stuff not just creation of new repos (so this may be treated as bug in master I think). I don't see any point in introducing separate DISABLE_MIRROR_CREATE parameter (overconfiguration?) but please feel free to add it if you need it.

@@ -28,16 +28,11 @@
<input id="auth_password" name="auth_password" type="password" value="{{.auth_password}}">
</div>

<div class="inline field">
<div class="inline field" {{if .DisableMirrors}} hidden{{end}}>
Copy link
Member

Choose a reason for hiding this comment

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

Can we just not render this field instead of hiding?

@pboguslawski
Copy link
Contributor Author

Was not sure if lack of elements in UI won't break backend (in case lack of variable in post will break something else). Feel free to remove rendering if you're sure it won't hurt.

@silverwind
Copy link
Member

Was not sure if lack of elements in UI won't break backend (in case lack of variable in post will break something else). Feel free to remove rendering if you're sure it won't hurt

A missing optional parameter must not break this and if it does, it needs to be fixed in the PR that removed it (this one).

Also there should be a server-side check to reject the parameter to prevent spoofing. It might already be there, haven't checked.

@pboguslawski
Copy link
Contributor Author

A missing optional parameter must not break this and if it does, it needs to be fixed in the PR that removed it (this one).

This PR only hides stuff and is backward compatible (same data is send to backend as before). It you need more changes feel free do extend this PR.

@stale
Copy link

stale bot commented Dec 19, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

@stale stale bot added the issue/stale label Dec 19, 2020
@lunny lunny added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Dec 19, 2020
@stale stale bot removed the issue/stale label Dec 19, 2020
pboguslawski added a commit to ibpl/gitea that referenced this pull request Feb 1, 2022
This mod fixes disabling unnecessary mirroring elements.

Related: go-gitea#16957
Related: go-gitea#13084
Author-Change-Id: IB#1105104
@pboguslawski
Copy link
Contributor Author

Replaced with #18527.

@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
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants