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

Give an option to disable Sync Fork #33264

Closed
noother opened this issue Jan 14, 2025 · 4 comments · Fixed by #33270
Closed

Give an option to disable Sync Fork #33264

noother opened this issue Jan 14, 2025 · 4 comments · Fixed by #33270
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@noother
Copy link

noother commented Jan 14, 2025

Feature Description

Hi,

the Sync Fork button is kind of like a red nuclear button at the moment.
It works and is very useful for sure, however it does not ask for any confirmation nor does it give any summary on what it would do - It just does it right away.

There are projects that started as a fork, but diverged in a way where it's not safe to just sync them with a push of the button, because it needs prerequisites, like changing SQL tables or requiring configs to be set in .gitignore'd files etc.

This is especially dangerous if the CI in the repo auto-deploys pushes to master into production systems.

Please either

  1. Give an option to disable the Sync Fork button globally, or for specific repos
  2. Ask for confirmation on clicking the Sync Fork button so it doesn't get clicked by accident (Bonus: With a diff / Pull Request like view on what it would do)
  3. As far as I understand it creates a Fake Pull Request and auto-merges it - Maybe only create the Pull Request without auto-merging it if possible. I think this might be the cleanest solution as this would enable one to cleanly check for external things that might have to be done.

Best Regards

Screenshots

No response

@noother noother added the type/proposal The new feature has not been accepted yet but needs to be discussed first. label Jan 14, 2025
@yp05327
Copy link
Contributor

yp05327 commented Jan 14, 2025

This is especially dangerous if the CI in the repo auto-deploys pushes to master into production systems.

I can not understand this. IIRC, fork repo does not have action unit.

There are projects that started as a fork, but diverged in a way where it's not safe to just sync them with a push of the button, because it needs prerequisites, like changing SQL tables or requiring configs to be set in .gitignore'd files etc.

Also, I can not understand this. Generally, it is used for GitHub Flow, sync the main branch of your fork (your workspace).
What is your use case that needs changing SQL tables or requiring configs to be set in .gitignore'd files?

@wxiaoguang
Copy link
Contributor

There are projects that started as a fork, but diverged in a way where it's not safe to just sync them with a push of the button, because it needs prerequisites, like changing SQL tables or requiring configs to be set in .gitignore'd files etc.

This is especially dangerous if the CI in the repo auto-deploys pushes to master into production systems.

In 1.23.1, the "sync fork" only syncs the base repo's default branch, is it still a problem? And if the repo diverged and there are conflicts, the sync will report a failure and won't do anything.

@wxiaoguang
Copy link
Contributor

As a quick (and backport-able) enhancement, we can do this: Add a confirm dialog for "sync fork" #33270

@noother
Copy link
Author

noother commented Jan 14, 2025

Hi,

since you asked, here's a little more detail on our use case:

Prerequisites:

  • There is a main repository
  • Individual projects are forked from this main repository using the "Branch to be cloned to the fork => main" setting
  • Changes for the project are done in this main branch. There is no other branch kept around that would mirror upstream's main branch.
  • The main branch of the forked repository is set up in a way that the production environment for this project is always instantly kept in sync with the main branch, via git --mirror in a post-receive hook. We're not using actions - This might not be how you'd do it, but it really doesn't matter, because even if actions are not triggered on Sync Fork (which they probably should?), they will be triggered on the next normal push to the main branch that follows.

Now the problem - Forget about SQL, lets focus on something more simple:

  • Upstream introduces a new mandatory config "jwt_secret_key" in a config.json file. It updates its config.json.example file as usual.
  • Somewhere in the code it says: if config.jwt_secret_key is not set: throw exception "jwt secret key is not set"
  • I now click the sync button, unaware right now of the new mandatory config, or I even click it by accident (because there's no confirmation)
  • There are no conflicts that would make the sync fail and the production environment syncs and breaks

#33270 is sufficient to prevent this.

It doesn't take the possibility of breaking due to unawareness (which a possibility to disable the button would), but it at least prevents accidentally breaking everything.

Right now I have to tell my Devs: "Hey, there's a cool new Sync Fork button. But no matter how curious you are, do not click it, also be careful to not click it by accident and watch your cat not jumping on the keyboard, as there is no confirmation and it would instantly break our live system"

With #33270 it's just "Hey, there's a cool new Sync Fork button, but don't click it without consulting Tech-Lead before" which is fine.

GiteaBot pushed a commit to GiteaBot/gitea that referenced this issue Jan 14, 2025
lafriks pushed a commit that referenced this issue Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants