Community
Participate
Working Groups
Created attachment 264148 [details] config EGit 4.4.1.201607150455-r I was working on the master branch of a local repo with the given config. In the Git Staging view, "Add Change-Id" was disabled, and the button on the left was called "Commit and Push...". When I clicked the button, the commit was pushed without any further questions. Every command whose name ends in an ellipsis (...) MUST open a dialog that allows the user to cancel without performing a persistent action. Either the button must be renamed in that case, or the Push Branch dialog can be opened that e.g. allows to chose a remote.
Still broken in 5.3.0. The ellipsis (...) must be removed from the "Commit and Push" button.
New Gerrit change created: https://git.eclipse.org/r/162769
(In reply to Markus Keller from comment #1) > The ellipsis (...) must be removed from the "Commit and Push" button. Instead the button now always shows a dialog. It's one more click but it gives the user a chance to review the push before it gets done.
Actually I find the dialog opened now unexpected and quite intimidating - I immediately worried I might push it somewhere wrong or change some config. I don't even want to think about the all options here when i just want to push something upstream. Especially the danger of changing the branch config somehow is worrisome. "Configure upstream for push and pull is preselected - I think this changes the config if push and pull are currently separate, right?. I rather cancelled it, went to the "Git Repositories" View right-clicked there on "Push to origin", which is annoying to do. What would make more sense here would be to show the "Push confirmation" dialog (that is should if you click "Preview") - and definitely no branch configuration be changed from here.
(In reply to Till Brychcy from comment #4) > dialog (that is should if you click "Preview") - and definitely no branch Should have been: "that is _shown_ if you click "Preview" (Missing an edit button for comments in bugzilla every day...)
restoring resolved status.
New Gerrit change created: https://git.eclipse.org/r/164934
(In reply to Eclipse Genie from comment #7) > New Gerrit change created: https://git.eclipse.org/r/164934 The proposed patch reverts the change. Such a disruptive change to the workflow of EGit should have been considered from an impact analysis. It completely destroys the seamless commit and push experience. If the goal is to prevent accidental pushes, a confirmation dialog with a preference sounds like a better way.
(In reply to Gunnar Wagenknecht from comment #8) > If the goal is to prevent accidental pushes, a confirmation dialog with a > preference sounds like a better way. The goal is to make the Commit And Push... action usable in case there is no upstream defined (this button was totally useless before the change, it was typically driving me to CLI many times a day while current behavior makes it work in all cases), and also to act as a confirmation dialog. If upstream is configured, the dialog is expected to act as a confirmation dialog as on 1 click or Enter keystroke should be necessary. If more action is required in case upstream is set, we need to improve that and remove need for extra action by pre-setting what's missing. A preference would "Always show this dialg when using Commit and Push" would indeed be a great addition. It could be shown both in dialog and preference page.
(In reply to Mickael Istria from comment #9) > If upstream is configured, the dialog is expected to act as a confirmation > dialog as on 1 click or Enter keystroke should be necessary. I believe this is not working as intended. Because when I tested this the dialog only showed a preview button. Additionally the force option was not checked by default. Thus, this change now requires me a lot more clicks whereas before no dialog showed up at all. Push just worked.
(In reply to Gunnar Wagenknecht from comment #10) > I believe this is not working as intended. Because when I tested this the > dialog only showed a preview button. Additionally the force option was not > checked by default. Thus, this change now requires me a lot more clicks > whereas before no dialog showed up at all. Push just worked. OK, it's indeed more clicks. However, making default focus on Finish instead of Preview or even enabling "force" by default seem like bad ideas to have by default on the dialog. So it seems to me we shouldn't change that. What would you think about having in the "Git Staging" view a checkbox "Skip Push dialog" checkbox to the left of "commit and push"?
No, please not more checkboxes or whatever there. In vertical layout, this area can get cramped already. I don't get why "force" should be checked by default at all. For the "Push HEAD..." case showing the dialog always is perfectly reasonable. For the "Commit and Push..." case we could indeed re-institute the previous behavior (pushing to the configured remote right away), but then we'll have to figure out beforehand whether the CommitJob will show a dialog, and we mustn't show the ellipsis on the button in that case. A preference for always showing the dialog also in the "Commit and Push..." case may make sense in that case, but that doesn't need a checkbox in the staging view. It would affect the non-Gerrit case only anyway; if it's a "Push to Gerrit", the button has always shown the wizard. Or, as an alternative to figuring it out beforehand and possibly a preference, maybe showing the dialog always but having "Push" (or "Finish") enabled and the default button would also work. At least it'd be not worse than the previous behavior, which just pushes somewhere without the user even seeing where. Then it should really be just one additional click or key press.
I think we should take the time to find the best resolution for this problem and release it in 5.8.1. I think it's not a showstopper and also I am attending the Eclipse board meeting the next 3 afternoons so I have no time left for a respin.
(In reply to Matthias Sohn from comment #13) > I think we should take the time to find the best resolution for this problem > and release it in 5.8.1. I think it's not a showstopper and also I am > attending the Eclipse board meeting the next 3 afternoons so I have no time > left for a respin. Matthias, can we revert the commit for 5.8.1 and come up with a better solution in 5.9?
(In reply to Thomas Wolf from comment #12) > No, please not more checkboxes or whatever there. In vertical layout, this > area can get cramped already. Agreed. > I don't get why "force" should be checked by default at all. Different workflow. GitHub PRs with personal branches and frequent rebase/amend. However, I'm not proposing to make it default. It works perfect without the dialog and push spec/config. > For the "Push HEAD..." case showing the dialog always is perfectly > reasonable. Greed. > > For the "Commit and Push..." case we could indeed re-institute the previous > behavior (pushing to the configured remote right away), but then we'll have > to figure out beforehand whether the CommitJob will show a dialog, and we > mustn't show the ellipsis on the button in that case. Agreed. > A preference for always showing the dialog also in the "Commit and Push..." > case may make sense in that case, but that doesn't need a checkbox in the > staging view. It would affect the non-Gerrit case only anyway; if it's a > "Push to Gerrit", the button has always shown the wizard. There is a suitable preference page: Preferences > Team > Git > Staging View [ ] Always show Push dialog (vs. only when necessary)
*** Bug 564371 has been marked as a duplicate of this bug. ***
I think the importance should be raised and the previous behavior should be reverted ASAP (or at least make it easily configurable)... using Commit and Push in the Staging view and in the dialog is basically unusable due to the additional required steps...
*** Bug 564691 has been marked as a duplicate of this bug. ***
*** Bug 564606 has been marked as a duplicate of this bug. ***
There already a couple duplicates reported now. I'd like to pursue and merge the review on 5.8 stable branch. 5.8.1. should restore the previous behavior. In 5.9 we can iterate on the current solution.
(In reply to Gunnar Wagenknecht from comment #20) > There already a couple duplicates reported now. I'd like to pursue and merge > the review on 5.8 stable branch. 5.8.1. should restore the previous > behavior. Although I'm not a committer, so my opinion doesn't really matter in the final choice of the committers; I'd like to mention that I would personally see this as a major regression in my workflow. It's been the first time in many years I've been able to click the "Commit and Push" button, and this drastically reduced how often I had to switch back to console (yes, to console because it's faster than switching between the various Eclipse views). IMO, reverting is the lazy path; it will bring a comparable amount of annoyance as the previous patch, but not to the same users; at least, everyone would have been irritated by EGit changes at least once. If someone is willing to put effort on this issue, they should instead work on a solution that fits all parties decently, as part of 5.8.1. I'll reopen bugs and crease more duplicates when this is reverted. You may have notice this issue has 2 votes and have had more "CC"s before it got fixed than the cumulated number of CCs on other bugs; so one may also interpret it as a sign that current behavior is quantitatively more desired than the previous one you intend to revert to. By the way, it may be better to use one of the duplicate issues for this discussion and close this one; as they are bascially talking about opposite things in summary, it's a mess to track the actual
(In reply to Mickael Istria from comment #21) > By the way, it may be better to use one of the duplicate issues for this > discussion and close this one; as they are bascially talking about opposite > things in summary, it's a mess to track the actual Actually, the proposal is to revert in 5.8.x. Thus, having this one open seems more natural to polish it's implementation. Per the rest of your arguments, I think it's ok to disagree on views. Although it fixes *your* workflow, it's is a poor implementation that breaks the workflow of others. Thus, it's better IMO to restore previous state and improve the implementation in main. I'm willing to make the behavior configurable by a preference, with the default being the old (pre 5.8) behavior.
New Gerrit change created: https://git.eclipse.org/r/c/egit/egit/+/166058
For me also a +1 reverting this (and applying the latest patch again) this is really horrible workflow. I am already this far that i just never use that "commit and push.." button anymore Because it is just to annoying that you go through a dialog and multiply steps what should just be 1 single click. Now i always do commit and then in the repository view, push that is at least faster then doing commit and push.. (i really hate all those modal dialogs) Dialog should only be shown when there is an error so we need to get the attention of the user. If there are some people that want this flow, then maybe this can be in the settings? That you want to have an extra push dialog?
Gerrit change https://git.eclipse.org/r/c/egit/egit/+/164934 was merged to [stable-5.8]. Commit: http://git.eclipse.org/c/egit/egit.git/commit/?id=8f437e520dad27f5b75d81547d1512a2ca19124b
I think it's clearer to reopen this one, as the summary shows that the revert actually "unresolve" the issue; and https://git.eclipse.org/r/c/egit/egit/+/166058 is the desired fix. I'll instead mark as resolved the ones (wrongly IMO) marked as duplicate.
(In reply to Mickael Istria from comment #26) > I think it's clearer to reopen this one, as the summary shows that the > revert actually "unresolve" the issue; and > https://git.eclipse.org/r/c/egit/egit/+/166058 is the desired fix. > I'll instead mark as resolved the ones (wrongly IMO) marked as duplicate. I marked the former duplicates to be no duplicates and set their target version to 5.8.1 which is now available on https://download.eclipse.org/egit/updates
New Gerrit change created: https://git.eclipse.org/r/c/egit/egit/+/166352
In nightly EGit I still cannot push from the staging view without additional dialog. Is that expected?
i think so because https://git.eclipse.org/r/c/egit/egit/+/166352 that needs to be merged now..
(In reply to Johan Compagner from comment #30) > i think so because https://git.eclipse.org/r/c/egit/egit/+/166352 that needs > to be merged now.. The review is https://git.eclipse.org/r/c/egit/egit/+/166058 Matthias requested a change. Looks like I won't be able to get to it this week, though. If anyone has cycle to pick it up please don't hesitate.
*** Bug 564784 has been marked as a duplicate of this bug. ***
Gerrit change https://git.eclipse.org/r/c/egit/egit/+/166058 was merged to [master]. Commit: http://git.eclipse.org/c/egit/egit.git/commit/?id=a046448df8dcf3f3387fe5ab38126c7a7a2f0ab9
(In reply to Eclipse Genie from comment #33) > Gerrit change https://git.eclipse.org/r/c/egit/egit/+/166058 was merged to > [master]. > Commit: > http://git.eclipse.org/c/egit/egit.git/commit/ > ?id=a046448df8dcf3f3387fe5ab38126c7a7a2f0ab9 Thanks Gunnar, highly appreciated that we finally can push again without an additional dialog.
Reverting this change is a major regression in functionality. The ability to check the "force push" checkbox directly from the "commit and push" wizard was a very welcome suprise in the 5.8 version. Given, the discussion above, I know it probably doesn't matter much to the powers that be, but I will not upgrade egit above 5.8.0.202006091008-r until this reversion is reverted or made into a configurable option. I have locked the Oomph setup for my 40 odd team of developers to 5.8.0.202006091008-r in order to keep the dialog.
(In reply to Missing name from comment #35) > but I > will not upgrade egit above 5.8.0.202006091008-r Your choice. > until this reversion is > reverted or made into a configurable option. Perhaps you should read the "New & Noteworthy" pages that come with each EGit release. In particular [1]. That page is, BTW, included in the built-in help pages for EGit. [1] https://wiki.eclipse.org/EGit/New_and_Noteworthy/5.9
(In reply to Thomas Wolf from comment #36) > (In reply to Missing name from comment #35) > > but I > > will not upgrade egit above 5.8.0.202006091008-r > > Your choice. > > > until this reversion is > > reverted or made into a configurable option. > > Perhaps you should read the "New & Noteworthy" pages that come with each > EGit release. In particular [1]. That page is, BTW, included in the built-in > help pages for EGit. > > [1] https://wiki.eclipse.org/EGit/New_and_Noteworthy/5.9 My apologies, I swear I crawled through the config preferences looking for something exactly like that especially after seeing reference to a preference mentioned in this very bug's comments. I must have been blind to the new setting somehow. I knew I should have waited 24 hours before giving into my inner grouch & posting a comment.
