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

Git: status bar lacks indication that it will push #8655

Closed
chrmarti opened this issue Jul 1, 2016 · 16 comments
Closed

Git: status bar lacks indication that it will push #8655

chrmarti opened this issue Jul 1, 2016 · 16 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug git GIT issues ux User experience issues verified Verification succeeded
Milestone

Comments

@chrmarti
Copy link
Contributor

chrmarti commented Jul 1, 2016

  • VSCode Version: Code - Insiders 1.3.0-insider (a2b9738, 2016-07-01T19:26:22.011Z)
  • OS Version: Darwin x64 15.5.0

Steps to Reproduce:

  1. Create commit, ready to be pushed
  2. Click on Git part of status bar
  3. -> Commit is pushed, which is unexpected
@chrmarti
Copy link
Contributor Author

chrmarti commented Jul 2, 2016

Not a regression (also in 1.2.1).

@bpasero bpasero added the git GIT issues label Jul 2, 2016
@joaomoreno
Copy link
Member

There are two actions in the status bar:

image

The first one will only list and let you check out branches. The second one is the sync action which will pull and push.

I am very reluctant in adding a confirmation dialog when clicking the sync action. If there are no better suggestions, I suggest to close this as designed.

@joaomoreno joaomoreno added the info-needed Issue requires more information from poster label Jul 4, 2016
@joaomoreno
Copy link
Member

Actually, #2397

@joaomoreno joaomoreno added the *duplicate Issue identified as a duplicate of another issue(s) label Jul 5, 2016
@chrmarti
Copy link
Contributor Author

chrmarti commented Jul 5, 2016

"Synchronize" isn't clear on what it will do. I initially assumed it would just fetch (or pull) and was surprised it pushed.

Is there telemetry on how often these actions are used? I'd guess that pull (and pull with rebase) are used more often than push or sync.

I don't favor a confirmation dialog either.

@joaomoreno
Copy link
Member

Since they are actions, we do have telemetry for it. Hey @seanmcbreen can we get some rough numbers here?

@chrmarti
Copy link
Contributor Author

@joaomoreno: I'm concerned about the first-timer experience. I was trying all actions in the status bar as part of our smoke test and didn't expect anything to happen without me being aware of it beforehand. I then inadvertently pushed a bogus change.

First-timers won't have the confirmation enabled. The confirmation doesn't help in that regard as long as it's disabled by default. I'm not necessarily suggesting to enable it by default, although that might be one option to avoid having this surprise.

@chrmarti
Copy link
Contributor Author

Also: "Synchronize" is not a well-understood term for someone familiar with git where "pull", "rebase", "merge", "push" are common.

@madjos
Copy link

madjos commented Jul 16, 2016

I have never seen an action that can be this destructive without a confirmation prompt.
No one on my team likes this sync button behavior. It does some weird combination of pull auto resolve and push. If I had ten votes I'd use them all to just remove this button altogether.
Right now it gets accidentally clicked by some team member and our entire branch gets into a messed up state. This is easily our least favorite part of an otherwise great ide.

@joaomoreno
Copy link
Member

@madjos It literally runs git pull followed by git push, no further weird magic.

There is now a "git.confirmSync" which your team can set to true that will cause a confirmation dialog to come up every time the action is clicked. This is already released in the Insiders build and will be released in the July stable release.

@joaomoreno
Copy link
Member

@chrmarti Agree with the synchronize term. I believe it was brought up 4 years ago when git wasn't known by most of our clients (Monaco) and we wanted a term people could relate to.

What we could do, once the work of programmatically changing the user settings is in, is to always prompt the user but give him a checkbox to never prompt him again. This choice would then be remembered in the git.confirmSync user setting.

@chrmarti
Copy link
Contributor Author

(Reopening for the discussion about the Sync action not properly indicating what it will do.)

Another option might be to replace the Sync action with the Push action, so the user's expectation are set correctly. To retain the current functionality when there are remote changes, the Push action could ask for confirmation to first Pull and then Push, but only if remote changes exist. This would have the advantage of only showing a confirmation dialog when it matters. CC: @egamma @chrisdias

@chrmarti chrmarti reopened this Jul 18, 2016
@chrmarti chrmarti removed *duplicate Issue identified as a duplicate of another issue(s) info-needed Issue requires more information from poster labels Jul 18, 2016
@aharpervc
Copy link

Could the button be redefined as "performs a configured series of git commands"? The default series of commands could be today's behavior. But then I could do what I want, which is, git stash 'vscode-sync', git fetch --all, git rebase, git stash pop, bailing out of any command fails. Note that my example here doesn't push, but also doesn't pull. I can't use today's button at all, but I'd love to be able to. Currently this process is a custom git alias (or, almost is).

@joaomoreno joaomoreno added this to the Backlog milestone Jul 18, 2016
@chrmarti chrmarti added the ux User experience issues label Jul 20, 2016
@joaomoreno joaomoreno added the bug Issue identified by VS Code Team member as probable bug label Aug 17, 2016
@joaomoreno
Copy link
Member

Fixed by 9601072

@roblourens roblourens added the verified Verification succeeded label Oct 26, 2016
@ramya-rao-a
Copy link
Contributor

@joaomoreno @chrmarti Am late to the party, but how about changing the tooltip from "Synchronize Changes" to "Pull and push changes" ?

@joaomoreno
Copy link
Member

cc @chrisdias @egamma @gregvanl on the wording

@chrmarti
Copy link
Contributor Author

Related: #13360 asks for renaming the command.

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug git GIT issues ux User experience issues verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants