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

[main] Update dependencies from dotnet/command-line-api #1826

Merged
merged 2 commits into from
Jun 15, 2023

Conversation

dotnet-maestro[bot]
Copy link
Contributor

@dotnet-maestro dotnet-maestro bot commented Apr 27, 2023

This pull request updates the following dependencies

From https://github.com/dotnet/command-line-api

  • Subscription: 0977a2e6-6141-4aac-1e76-08d967f4d98f
  • Build: 20230419.2
  • Date Produced: April 19, 2023 11:56:36 PM UTC
  • Commit: 87704ce036fb23a4174b8290f249706aa35ab255
  • Branch: refs/heads/main

…uild 20230419.2

System.CommandLine , System.CommandLine.Rendering
 From Version 2.0.0-beta4.22564.1 -> To Version 2.0.0-beta4.23219.2
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Auto-approve

@sharwell
Copy link
Member

sharwell commented May 1, 2023

I requested a review from @jonsequitur

@sharwell
Copy link
Member

sharwell commented May 1, 2023

@baronfel @adamsitnik would also be able to review this

internal static readonly CliOption<string> BinarylogOption = new CliOption<string>("--binarylog")
{
HelpName = "binary-log-path",
Arity = ArgumentArity.ZeroOrOne,

Choose a reason for hiding this comment

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

In general, you shouldn't need to set the arity unless there's some custom parsing going on. Otherwise, it's inferred from the generic type of the Option<T> or Argument<T>.

Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

Overall the changes from @sharwell look great, the only thing we need is to sync the version with dotnet/sdk#33157.

@@ -5,13 +5,13 @@
<Uri>https://github.com/dotnet/roslyn</Uri>
<Sha>d7a4cad21c39e18f3d5e1f7fa7dd3f93668066b4</Sha>
</Dependency>
<Dependency Name="System.CommandLine" Version="2.0.0-beta4.22564.1">
<Dependency Name="System.CommandLine" Version="2.0.0-beta4.23219.2">
Copy link
Member

Choose a reason for hiding this comment

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

We need to bump the version to be in sync with dotnet/sdk#33157

Suggested change
<Dependency Name="System.CommandLine" Version="2.0.0-beta4.23219.2">
<Dependency Name="System.CommandLine" Version="2.0.0-beta4.23307.1">

</Dependency>
<Dependency Name="System.CommandLine.Rendering" Version="0.4.0-alpha.22564.1">
<Dependency Name="System.CommandLine.Rendering" Version="0.4.0-alpha.23219.2">
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<Dependency Name="System.CommandLine.Rendering" Version="0.4.0-alpha.23219.2">
<Dependency Name="System.CommandLine.Rendering" Version="0.4.0-alpha.23307.1">

Comment on lines +20 to +21
<SystemCommandLineVersion>2.0.0-beta4.23219.2</SystemCommandLineVersion>
<SystemCommandLineRenderingVersion>0.4.0-alpha.23219.2</SystemCommandLineRenderingVersion>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<SystemCommandLineVersion>2.0.0-beta4.23219.2</SystemCommandLineVersion>
<SystemCommandLineRenderingVersion>0.4.0-alpha.23219.2</SystemCommandLineRenderingVersion>
<SystemCommandLineVersion>2.0.0-beta4.23307.1</SystemCommandLineVersion>
<SystemCommandLineRenderingVersion>0.4.0-alpha.23307.1</SystemCommandLineRenderingVersion>

<Uri>https://github.com/dotnet/command-line-api</Uri>
<Sha>350a618ab44932c568ae2392d7a571281baed59a</Sha>
<Sha>87704ce036fb23a4174b8290f249706aa35ab255</Sha>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<Sha>87704ce036fb23a4174b8290f249706aa35ab255</Sha>
<Sha>02fe27cd6a9b001c8feb7938e6ef4b3799745759</Sha>

<Uri>https://github.com/dotnet/command-line-api</Uri>
<Sha>350a618ab44932c568ae2392d7a571281baed59a</Sha>
<Sha>87704ce036fb23a4174b8290f249706aa35ab255</Sha>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<Sha>87704ce036fb23a4174b8290f249706aa35ab255</Sha>
<Sha>02fe27cd6a9b001c8feb7938e6ef4b3799745759</Sha>

@adamsitnik
Copy link
Member

I don't have the write rights here hence I cannot apply my suggestion or solve the merge conflict

image

@sharwell is there any chance you could bump the S.CL version and solve the merge conflict?

@adamsitnik
Copy link
Member

adamsitnik commented Jun 12, 2023

nvm I've sent #1872

@JoeRobich JoeRobich merged commit 51b963f into main Jun 15, 2023
@ghost ghost added this to the Next milestone Jun 15, 2023
@dotnet-maestro dotnet-maestro bot deleted the darc-main-e24473e8-6950-4d67-aabf-025da8dc8ea2 branch June 15, 2023 19:25
@baronfel
Copy link
Member

Hey @JoeRobich can you revert this? we need to syncronize all the S.CL updates across roslyn/format/runtime/aspnet/sdk/templating and sequence them all together, and since we're getting close to p6 snap we were going to hold until after that snap occurred next week, to prevent too much churn in case we have to back it out across those repos.

@JoeRobich
Copy link
Member

I knew I should have kept out of it. Will do.

@baronfel
Copy link
Member

No good deed goes unpunished :) Thanks!

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.

5 participants