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

Reduce steps needed to reset workspace #860

Merged
merged 8 commits into from
Sep 30, 2021
Merged

Conversation

mattseddon
Copy link
Member

@mattseddon mattseddon commented Sep 30, 2021

This PR introduces a Reset the Workspace command in place of what was Checkout at the root of each SCM view. This command goes further than the previous one by first running git reset --hard HEAD & git clean -f -d -q to remove all of the current git changes in the workspace, it then runs dvc checkout to clear out any remaining difference between HEAD and what is in the workspace. The user will not have to understand / hold this concept in their head, they just go back to a clean workspace. LMK what you think on this one.

Demo:

Screen.Recording.2021-09-30.at.12.43.33.pm.mov
Screen.Recording.2021-09-30.at.1.10.55.pm.mov

Note: I have a strong feeling that we should make this command available from the command palette which means I will have to introduce a Repositories class which will look a lot like the Experiments one.

Relates to #609

@mattseddon mattseddon added the product PR that affects product label Sep 30, 2021
@mattseddon mattseddon self-assigned this Sep 30, 2021
const response = await getWarningResponse(
'Are you sure you want to discard ALL workspace changes?\n' +
'This is IRREVERSIBLE!\n' +
'Your current working set will be FOREVER LOST if you proceed.',
Copy link
Member Author

Choose a reason for hiding this comment

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

[F] This pretty much matches what the git extension throws up. You can see a comparison in the first demo.

expect(mockGitReset).to.be.calledWith({
args: ['clean', '-f', '-d', '-q'],
cwd: dvcDemoPath,
executable: 'git'
Copy link
Member Author

Choose a reason for hiding this comment

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

[F] Definitely do not want to be calling these actions in tests

executable: 'git'
})
await executeProcess({
args: ['clean', '-f', '-d', '-q'],
Copy link
Member Author

Choose a reason for hiding this comment

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

[F] We need this to remove untracked from the workspace.

Copy link
Member Author

Choose a reason for hiding this comment

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

this mirrors what the git extension does

"when": "scmProvider == dvc && dvc.commands.available == true"
},
{
"command": "dvc.checkout",
Copy link
Member Author

Choose a reason for hiding this comment

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

[F] This now goes into a new context menu which we can build on.

@mattseddon mattseddon marked this pull request as ready for review September 30, 2021 03:40
@rogermparent
Copy link
Contributor

I'll admit on first read I found it a little scary how nuclear this is, but it seems like a totally viable workflow that users will appreciate, and considering the Git extension does something similar we should be in the clear.

@mattseddon mattseddon enabled auto-merge (squash) September 30, 2021 21:58
@codeclimate
Copy link

codeclimate bot commented Sep 30, 2021

Code Climate has analyzed commit d60e25a and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (85% is the threshold).

This pull request will bring the total coverage in the repository to 96.5% (0.0% change).

View more on Code Climate.

@mattseddon mattseddon merged commit 8673fd7 into master Sep 30, 2021
@mattseddon mattseddon deleted the update-checkout-action branch September 30, 2021 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product PR that affects product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants