jesseduffield / lazygit

simple terminal UI for git commands
MIT License
53.65k stars 1.87k forks source link

Add config option to disable push to main branches #4054

Open trskare opened 2 weeks ago

trskare commented 2 weeks ago
stefanhaller commented 2 weeks ago

Can you explain the motivation for this? Usually this is configured on the server (e.g. in github or gitlab), not in clients. Why doesn't that work for you?

trskare commented 2 weeks ago

Can you explain the motivation for this? Usually this is configured on the server (e.g. in github or gitlab), not in clients. Why doesn't that work for you?

https://github.com/jesseduffield/lazygit/issues/3716

stefanhaller commented 2 weeks ago

That doesn't explain anything. The comments on that issue support my point that this should be configured on the server. What's the justification for adding an additional warning in the client?

trskare commented 2 weeks ago

That doesn't explain anything. The comments on that issue support my point that this should be configured on the server. What's the justification for adding an additional warning in the client?

Yes it should be configured on the server, but its a safety measure option incase someone has temporarily disabled or forgot to enabled it on the server side etc.

Pressing the P key on a main branch in lazygit is a mistake that is a lot of easier to make than to write "git push origin/main". When you work with repos controlling a lot of stuff with big consequences its all about minimising risk.

mark2185 commented 2 weeks ago

I'd also suggest to unset the upstream for those branches.

If you need to update them, set the upstream, fetch, unset upstream, it requires more discipline, but it's a solution.

trskare commented 2 weeks ago

I'd also suggest to unset the upstream for those branches.

If you need to update them, set the upstream, fetch, unset upstream, it requires more discipline, but it's a solution.

Yeah, that will prompt you with a "Enter upstream" dialogbox when you try to push, not exactly a warning.

I was just trying to be helpful to create a PR for my issue, but I can always have my own fork of lazygit with it if it isn´t wanted.

stefanhaller commented 2 weeks ago

@trskare I'm sorry if I sounded dismissive, or if my tone came across as overly unfriendly, that wasn't my intention. Apologies for that, I'm sometimes not very sensitive to that.

My main reason for being skeptical about this addition is that I'm generally reluctant to add new user config options. We have too many already, and that's a problem because it makes it harder to find the really useful ones among the hundreds that only one user wants[^1]. At some point, people will stop reading docs/Config.md if it has too many pages. It's already too long today for my taste.

I'll let @jesseduffield make the call on this; my vote is still for not adding the option, since with a properly configured server it's not needed.

[^1]: I have to admit that I'm guilty of adding options that probably only I use, e.g. gui. scrollOffBehavior.

jesseduffield commented 2 weeks ago

I don't support an option to disable pushing to a main branch, because you can and should make it a protected branch on the server.

For the record, I could be persuaded to support an option to warn upon pushing to a main branch . The use case is this: you have a repo with a protected main branch but you're an admin so you're allowed to push to it if you consider it necessary, but you still want to add some friction to prevent accidentally pushing to it. I'm actually in this boat myself in a few repos.

But that's somewhat different to what this PR is doing.

trskare commented 2 weeks ago

@stefanhaller its ok! no worries!

@jesseduffield a warning instead sounds fair, ill see if i manage to make that instead.