Skip to content

Adds a way to force push from the Graph#3507

Merged
d13 merged 7 commits intomainfrom
features/3493-add-a-way-to-force-push-from-the-graph
Oct 2, 2024
Merged

Adds a way to force push from the Graph#3507
d13 merged 7 commits intomainfrom
features/3493-add-a-way-to-force-push-from-the-graph

Conversation

@nzaytsev
Copy link
Contributor

@nzaytsev nzaytsev commented Sep 2, 2024 •
edited
Loading

Description

  • add split button dropdown in case of both ahead and behind changes appeared

Screenshot 2024-09-04 at 14 58 43

Screenshot 2024-09-04 at 14 58 50

Checklist

  • I have followed the guidelines in the Contributing document
  • My changes follow the coding style of this project
  • My changes build without any errors or warnings
  • My changes have been formatted and linted
  • My changes include any required corresponding changes to the documentation (including CHANGELOG.md and README.md)
  • My changes have been rebased and squashed to the minimal number (typically 1) of relevant commits
  • My changes have a descriptive commit message with a short title, including a Fixes $XXX - or Closes #XXX - prefix to auto-close the issue that your PR addresses

@nzaytsev nzaytsev linked an issue Sep 2, 2024 that may be closed by this pull request
Add a way to force push from the Graph #3493
Closed
@nzaytsev
Copy link
Contributor Author

nzaytsev commented Sep 2, 2024

@d13 @eamodio @sergeibbb
Check this behavior, I suggested the sync button that is similar to default VSCode button. It appears only when the branch is both ahead and behind and suggests pulling with options, then awaits and suggests pushing with available options. Also it contains the split button dropdown with 3 options: sync, pull and push

@nzaytsev nzaytsev marked this pull request as ready for review September 2, 2024 04:57
@d13
Copy link
Member

d13 commented Sep 3, 2024

@d13 @eamodio @sergeibbb Check this behavior, I suggested the sync button that is similar to default VSCode button. It appears only when the branch is both ahead and behind and suggests pulling with options, then awaits and suggests pushing with available options. Also it contains the split button dropdown with 3 options: sync, pull and push

Let's wait on the sync action and focus on the push, pull and force-push. Sync will need some more thought on how we offer that, both on the Graph and throughout GitLens.

d13
d13 reviewed Sep 3, 2024
View reviewed changes
Copy link
Member

@d13 d13 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

In general, we want to avoid using React as much as possible in GitLens.

For history, when the graph view was originally built, we didn't have a grasp on how to effectively use web components yet.

Our plan now is to invert how React is used today in the graph view. Instead of creating React wrappers for every web component used in the Graph, we plan to put a single web component wrapper around the graph's react component and focus on using web platform APIs for reactivity.

@nzaytsev
Copy link
Contributor Author

nzaytsev commented Sep 4, 2024

removed/hidden code associated with sync function, use only pull and push

@d13

In general, we want to avoid using React as much as possible in GitLens.

Do you suggest moving the SyncButton component to lit now?

@nzaytsev nzaytsev force-pushed the features/3493-add-a-way-to-force-push-from-the-graph branch from 962de6a to b105be2 Compare September 9, 2024 09:33
@nzaytsev nzaytsev requested a review from d13 September 9, 2024 09:34
@d13 d13 force-pushed the features/3493-add-a-way-to-force-push-from-the-graph branch 2 times, most recently from f87b918 to c6121ad Compare October 1, 2024 20:26
@d13 d13 force-pushed the features/3493-add-a-way-to-force-push-from-the-graph branch from c6121ad to a587e13 Compare October 2, 2024 14:07
eamodio
eamodio approved these changes Oct 2, 2024
View reviewed changes
Copy link
Member

@eamodio eamodio 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, just pushed an enhancement to the tooltip

@d13 d13 merged commit 6211006 into main Oct 2, 2024
@eamodio eamodio deleted the features/3493-add-a-way-to-force-push-from-the-graph branch October 14, 2024 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eamodio eamodio eamodio approved these changes

@sergeibbb sergeibbb Awaiting requested review from sergeibbb

@d13 d13 Awaiting requested review from d13

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add a way to force push from the Graph

3 participants

@nzaytsev @d13 @eamodio