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

Corrected default output directory for dotnet publish #5500

Merged
merged 1 commit into from
May 21, 2018

Conversation

svick
Copy link
Contributor

@svick svick commented May 20, 2018

Fixes #5309.

To verify this is indeed the behavior of dotnet publish, I looked at the source code (cli, sdk) and also tried running the command myself.

@svick svick requested a review from mairaw as a code owner May 20, 2018 16:23
@dasMulli
Copy link
Contributor

If we want to be super-correct the ./ beginning may be misleading as the path is relative to the project and the ./ part in docs may trick readers into thinking it was relative to the command.
(I'm still hoping to fix that for a 3.0 SDK since the breaking change was too late for 2.0 - dotnet/cli#7166).

Does that make sense or is it too overthinking?

@svick
Copy link
Contributor Author

svick commented May 21, 2018

@dasMulli Instead of trying to take attention away from the fact that the default path is relative, maybe it would make more sense to make it clearer that the following sentence applies to the default path too?

If a relative path is provided, the output directory generated is relative to the project file location, not to the current working directory.

I'm not sure what would be a good wording for that. Maybe removing the "provided" part would be enough?

If the path is relative, the output directory generated is relative to the project file location, not to the current working directory.

Copy link
Member

@BillWagner BillWagner left a comment

Choose a reason for hiding this comment

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

Thanks for making this change @svick

I've reviewed the changes, and the CLI source in your comment, and I'll :shipit: now.

Thanks again for your contribution, and thanks @dasMulli for adding his review.

@BillWagner BillWagner merged commit c7e26f6 into dotnet:master May 21, 2018
@svick svick deleted the patch-5 branch May 21, 2018 19:27
@Mike-E-angelo
Copy link

Cooooool... so there was something going on here. Thank you all for your impressive efforts! It's really exciting to see everything work together and to even be a part of it. 👍

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.

4 participants