Skip to content

Conversation

@mattseddon
Copy link
Contributor

@mattseddon mattseddon commented Aug 22, 2022

1/2 main <- this <- #2226

This PR moves all calls made to Git into the DVC output channel. This will make the user aware of these calls and should let them debug should something go wrong.

Demo

Screen.Recording.2022-08-22.at.1.10.50.pm.mov

@mattseddon mattseddon added the product PR that affects product label Aug 22, 2022
@mattseddon mattseddon self-assigned this Aug 22, 2022
@mattseddon mattseddon force-pushed the add-git-cli-classes branch from bfd51fc to e818412 Compare August 22, 2022 02:58
@mattseddon mattseddon force-pushed the add-git-cli-classes branch from e818412 to e08cf2c Compare August 22, 2022 02:59
export const GIT_LOGS_REFS = join(DOT_GIT, 'logs', 'refs')
export const HEADS_GIT_REFS = join(GIT_REFS, 'heads')

const getUris = (repositoryRoot: string, relativePaths: string[]) =>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[F] These are the all the functions that got moved into the new classes.

getStageAllCommand(repositories)
internalCommands.registerExternalCliCommand<Root>(
RegisteredCliCommands.GIT_STAGE_ALL,
getStageAllCommand(repositories, internalCommands)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[F] The extension will now "offer to show errors" (open the output channel) if these commands fail.

@mattseddon mattseddon marked this pull request as ready for review August 22, 2022 06:55
@mattseddon mattseddon requested a review from shcheklein August 22, 2022 06:55
@mattseddon mattseddon changed the title Add git cli calls into output channel Add Git cli calls into output channel Aug 22, 2022
@mattseddon
Copy link
Contributor Author

Will have to integrate with #2091

const mockCheckout = stub(CliExecutor.prototype, 'checkout').resolves('')
const mockGitReset = stub(ProcessExecution, 'executeProcess').resolves('')
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const mockExecuteProcess = stub(Cli.prototype as any, 'executeProcess')
Copy link
Contributor

Choose a reason for hiding this comment

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

Move to a util so that we only have one elint-disable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll sort this out for good by adding a test util as a follow-up today.

@qlty-cloud-legacy
Copy link

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

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

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

View more on Code Climate.

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