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

Removed dependency to Microsoft.Dotnet.Cli.Utils #5482

Merged
merged 7 commits into from
Nov 3, 2022

Conversation

vlada-shubina
Copy link
Member

Problem

Solution

Created separated CommandUtils package usable from CLI and XUnit tests.
Removed dependency to Microsoft.Dotnet.Cli.Utils.

Checks:

  • Added unit tests
  • Added #nullable enable to all the modified files ?

@vlada-shubina vlada-shubina requested a review from a team as a code owner October 20, 2022 10:33
@vlada-shubina
Copy link
Member Author

@JanKrivanek I excluded the tools from source-build in the last commit. If they are needed, we can turn it on again.

@vlada-shubina vlada-shubina force-pushed the remove-cli-utils branch 2 times, most recently from 3de3b2a to a7988b1 Compare October 24, 2022 10:37
@vlada-shubina vlada-shubina marked this pull request as draft October 24, 2022 10:57
@vlada-shubina
Copy link
Member Author

we agreed not to create a public package at the time, and include the source code internally instead.

@vlada-shubina
Copy link
Member Author

@JanKrivanek as agreed - moved the utils to shared internal code.
It might get too difficult to review - let me know if you'd like to split it into 2 PRs.

Copy link
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

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

Overall looks good.
I have 2 minor things for considerations - but not strongly opinionated about those.

I'm very much looking forward for this to be merged!! :-) As it will simplify my life in unblocking dogfooding in sdk (need to be able to inject custom environemnt and dotnet assembly)

@vlada-shubina vlada-shubina enabled auto-merge (rebase) November 3, 2022 16:26
@vlada-shubina
Copy link
Member Author

/backport to release/7.0.2xx

@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2022

Started backporting to release/7.0.2xx: https://github.com/dotnet/templating/actions/runs/3387467177

@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2022

@vlada-shubina backporting to release/7.0.2xx failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: removed dependency on Cli.Utils
Using index info to reconstruct a base tree...
M	eng/Versions.props
Falling back to patching base and 3-way merge...
Removing test/Microsoft.TemplateEngine.TestHelper/Commands/SdkCommandSpec.cs
Removing test/Microsoft.TemplateEngine.TestHelper/Commands/DotnetCommand.cs
Removing src/Microsoft.TemplateEngine.Authoring.TemplateVerifier/Commands/TestCommand.cs
Removing src/Microsoft.TemplateEngine.Authoring.TemplateVerifier/Commands/DotnetCommand.cs
Removing src/Microsoft.TemplateEngine.Authoring.TemplateVerifier/Commands/BasicCommand.cs
Auto-merging eng/Versions.props
CONFLICT (content): Merge conflict in eng/Versions.props
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 removed dependency on Cli.Utils
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

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.

Consider dropping dependency to Microsoft.DotNet.Cli.Utils.
3 participants