-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add support for new output path format #29599
Changes from all commits
2ea0569
812f8a5
0d8ed7f
05aba59
dc4f379
046986d
0d3fe14
09a2541
1145160
0d117bf
27ae9eb
c5be0db
2462691
9b0dcd2
63df854
dca2d3a
31e613d
f7de61d
29d3685
c4e3545
aac09cc
b07165d
2026b9a
fc0cb10
81bf80e
06fe5ab
de35c57
3a9f255
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,6 +51,15 @@ public static Option<string> FrameworkOption(string description) => | |
}.ForwardAsSingle(o => $"-property:TargetFramework={o}") | ||
.AddCompletions(Complete.TargetFrameworksFromProjectFile); | ||
|
||
public static Option<string> ArtifactsPathOption = | ||
new ForwardedOption<string>( | ||
// --artifacts-path is pretty verbose, should we use --artifacts instead (or possibly support both)? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like supporting both. We should ideally mention this in the doc's options for each command, such as in https://learn.microsoft.com/en-us/dotnet/core/tools/dotnet-build There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't artifacts imply the produced output instead of their location? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's true. Nvm on the --artifacts path, still think we should include this in the docs tho! Kind of unfortunate -a, -o is taken, but not sure if this needs that level of 'visibility' |
||
new string[] { "--artifacts-path" }, | ||
description: CommonLocalizableStrings.ArtifactsPathOptionDescription) | ||
{ | ||
ArgumentHelpName = CommonLocalizableStrings.ArtifactsPathArgumentName | ||
}.ForwardAsSingle(o => $"-property:ArtifactsPath={CommandDirectoryContext.GetFullPath(o)}"); | ||
|
||
private static string RuntimeArgName = CommonLocalizableStrings.RuntimeIdentifierArgumentName; | ||
public static IEnumerable<string> RuntimeArgFunc(string rid) | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this include intermediate files too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does include intermediate files. I didn't include that in the description because I don't expect people to be looking for the intermediate path in order to do something with the files there.
Note that if you set the
ArtifactsPath
property in the project file instead of in Directory.Build.props or on the command line, that it doesn't affect the intermediate output path.