Correctly request force-pushing in a triangular workflow #3528
Conversation
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesCodacy will stop sending the deprecated coverage status from June 5th, 2024. Learn more Footnotes
|
stefanhaller
force-pushed
the
support-triangular-workflow
branch
from
45aa829
to
3073e53
Compare
| @@ -20,8 +21,16 @@ func NewSessionStateLoader(c *helpers.HelperCommon, refsHelper *helpers.RefsHelp | |||
| } | |||
| } | |||
|
|
|||
| type Commit struct { | |||
| Hash string | |||
| // We create shims for all the model classes in order to get a more stable API | |||
There was a problem hiding this comment.
Great stuff. We should also update docs/Custom_Command_Keybindings.md which currently says
To see what fields are available on e.g. the
SelectedFile, see here
We should update this to just point to this file now
There was a problem hiding this comment.
This is a tricky one. In some cases the original model struct has documentation for its fields (e.g. models.Branch), which I didn't copy over to the shims. So pointing users to the shims gives them less information.
Or should I copy the comments over? Seemed so redundant.
There was a problem hiding this comment.
hmm, interesting. I say point the users to this file but don't copy the comments. Most fields are self-explanatory and the ones that aren't are likely not the ones the user needs
There was a problem hiding this comment.
OK, works for me. To make this easier for users, I decided to move the struct definitions to a separate file (7ab7fae), and to remove the "Shim" from the names again (416106a). Documentation is adapted here: 91132a7.
I noticed that the documentation already says that we don't guarantee backwards compatibility. Do we now, and should remove this sentence? Also, it says we are planning to expose a tighter API in the future; do we want to change anything about that sentence? Feel free to push a fixup if you want to tweak that documentation further.
There was a problem hiding this comment.
I suppose we now do support backwards compatibility so indeed we can lose that line. We can also remove the part about the tighter API: I have no plans for changing things anytime soon.
There was a problem hiding this comment.
OK, removed in 283cf3a.
Going to squash now, and then merge (will take a moment as the fixups conflict).
stefanhaller
force-pushed
the
support-triangular-workflow
branch
2 times, most recently
from
91132a7
to
283cf3a
Compare
This guards against accidentally renaming a model field and thereby breaking user's custom commands. With this change we'll get a build failure when we do that.
It is unexpected that a function called PushBranch also sets the upstream branch; also, we want to add a PushBranch function in the next commit that doesn't.
… workflow Our code doesn't realize that we need to prompt the user to force push, when the branch is up-to-date with its upstream but not with the branch that we're pushing to.
In preparation for adding AheadForPush/BehindForPush in the next commit.
In a triangular workflow the branch that you're pulling from is not the same as the one that you are pushing to. For example, some people find it useful to set the upstream branch to origin/master so that pulling effectively rebases onto master, and set the push.default git config to "current" so that "feature" pushes to origin/feature. Another example is a fork-based workflow where "feature" has upstream set to upstream/main, and the repo has remote.pushDefault set to "origin", so pushing on "feature" pushes to origin/feature. This commit adds new fields to models.Branch that store the ahead/behind information against the push branch; for the "normal" workflow where you pull and push from/to the upstream branch, AheadForPush/BehindForPush will be the same as AheadForPull/BehindForPull.
To determine whether we need to ask for force pushing, we need to query the push branch rather than the upstream branch, in case they are not the same.
stefanhaller
force-pushed
the
support-triangular-workflow
branch
from
283cf3a
to
c5cf1b2
Compare
Some people push to a different branch (or even remote) than they pull from. One example is described in #3437. Our logic of when to request a force push is not appropriate for these workflows: we check the configured upstream branch for divergence, but that's the one you pull from. We should instead check the push-to branch for divergence.
Fixes #3437.