Change the default value of update_ref force to false
#980
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Problem ==== Currently, `Octokit::Client#update_ref` updates a reference with `force: true` option. It works like `git push --force`. I think the behaviour is a problem for two reasons. Firstly, the behaviour is dangerous. Force push destroys pushed commits sometimes. I think dangerous operations should be optional. Secondly, the default value is false in the GitHub API. > Indicates whether to force the update or to make sure the update is a fast-forward update. Leaving this out or setting it to false will make sure you're not overwriting work. Default: false > > https://developer.github.com/v3/git/refs/#update-a-reference I think Octokit default value should be same as the GitHub API. Note === It is a breaking change. If someone uses `update_ref` method without a default value and expects "force push", it will be broken. But I think we should change default behaviour. What do you think?
|
Thanks for the PR @pocke! I like this change, but like you said it's a breaking change to the client. So we're going to wait to 🚢 this when we start the process of releasing Octokit 5.0. Let me know if you have any questions. |
It seems good. Thank you! |
oranday21
approved these changes
Jun 19, 2018
kfcampbell
approved these changes
Oct 18, 2022
There was a problem hiding this comment.
I love the idea of matching the GitHub API's defaults, and we recently merged in another breaking change in #1495, so I think this is an opportune time to get this change in.
nickfloyd
added
Type: Breaking change
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Currently,
Octokit::Client#update_refupdates a reference withforce: trueoption. It works likegit push --force.I think the behaviour is a problem for two reasons.
Firstly, the behaviour is dangerous.
Force push destroys pushed commits sometimes. I think dangerous operations should be optional.
Secondly, the default value is false in the GitHub API.
I think Octokit default value should be same as the GitHub API.
Note
It is a breaking change. If someone uses
update_refmethod without a default value and expects "force push", it will be broken.But I think we should change default behaviour. What do you think?