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

Add merge squash action #1267

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

ylecuyer
Copy link
Contributor

This PR adds the merge squash action to the graph. I'm waiting for your feedback before writing tests :)

merge-squash

Copy link
Collaborator

@campersau campersau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and maybe easier for some to understand then doing it the other way around via Squash.
Is it possible to highlight the commits which are going to be merge squashed like in Squash?

Squash:
image

Merge Squash:
image

components/graph/git-graph-actions.js Outdated Show resolved Hide resolved
@Hirse
Copy link
Contributor

Hirse commented Feb 3, 2020

What is the difference in the result between "Squash" and "Merge Squash"?

If the outcome is the same, I would be in favor of having the name, color, and icon of the button also be the same.

@ylecuyer
Copy link
Contributor Author

Looks good and maybe easier for some to understand then doing it the other way around via Squash.
Is it possible to highlight the commits which are going to be merge squashed like in Squash?

Squash:
image

Merge Squash:
image

done

What is the difference in the result between "Squash" and "Merge Squash"?

It is the same if you select the right nodes. But I prefer to have the proposed named in the PR as this is the common wording used to describe this action, see for example the github button :

select-squash-and-merge-from-drop-down-menu

@Hirse
Copy link
Contributor

Hirse commented Feb 10, 2020

So, if I understand correctly, the new "Merge Squash" does the same as the existing "Squash" on the newest (topmost) commit?

  1. Should we change it to "Squash Merge" then?
  2. Should we change the previous "Squash" to "Squash Merge" too then?

@campersau
Copy link
Collaborator

After thinking more about the UI I also think @Hirse renames suggestions would make sense. Also it might be good to unify the two squash actions even further by using the same color and icon.

Here is my suggestion:
image

image

I have picked the Squash & Merge spelling because we already have Commit & Push elsewhere.

What do you/others think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants