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

3533 Change reset terminal-run commands into normal commands w/ proper error handling #3676

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

sergiolms
Copy link
Contributor

@sergiolms sergiolms commented Oct 15, 2024

Description

This task creates the git reset commands to move away from runTerminalCommand. Solves #3533

To be defined:

  • Add those error cases where we want to recover from

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

@sergiolms sergiolms self-assigned this Oct 15, 2024
@sergiolms sergiolms marked this pull request as ready for review October 15, 2024 14:29
Copy link
Contributor

@axosoft-ramint axosoft-ramint left a comment

Choose a reason for hiding this comment

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

Build errors:

image

src/env/node/git/git.ts Outdated Show resolved Hide resolved
src/env/node/git/git.ts Outdated Show resolved Hide resolved
src/env/node/git/localGitProvider.ts Show resolved Hide resolved
@sergiolms sergiolms force-pushed the feature/#3533_change_git_reset_cmds_from_terminal_to_normal_cmds branch from e6953f6 to 94bda2c Compare October 23, 2024 12:50
Copy link
Contributor

@axosoft-ramint axosoft-ramint left a comment

Choose a reason for hiding this comment

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

Tested with "reset current branch to tip" graph context menu command, and it LGTM. Let me know if I should test other cases.

@sergiolms sergiolms force-pushed the feature/#3533_change_git_reset_cmds_from_terminal_to_normal_cmds branch 2 times, most recently from 3adc472 to 1a652e4 Compare November 7, 2024 13:09
Copy link
Contributor

@axosoft-ramint axosoft-ramint left a comment

Choose a reason for hiding this comment

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

Build errors on the latest version:

image

Reason below.

src/env/node/git/git.ts Show resolved Hide resolved
@axosoft-ramint
Copy link
Contributor

@sergiolms I went ahead and fixed the ☝🏼 error

Copy link
Contributor

@axosoft-ramint axosoft-ramint left a comment

Choose a reason for hiding this comment

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

I couldn't reproduce any of the error types here, but I don't have further time to test. @eamodio feel free to give this a test run, otherwise LGTM

@sergiolms sergiolms force-pushed the feature/#3533_change_git_reset_cmds_from_terminal_to_normal_cmds branch from 1187eac to cbef015 Compare November 8, 2024 15:03
@sergeibbb sergeibbb requested review from sergeibbb and removed request for sergeibbb November 14, 2024 18:12
@sergiolms sergiolms force-pushed the feature/#3533_change_git_reset_cmds_from_terminal_to_normal_cmds branch from cbef015 to e6634de Compare November 21, 2024 14:43
@sergiolms sergiolms force-pushed the feature/#3533_change_git_reset_cmds_from_terminal_to_normal_cmds branch from e6634de to 7e6d1cf Compare November 27, 2024 14:27
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.

2 participants