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

Add extra options to dotnet pack command #9956

Merged
merged 9 commits into from
Apr 20, 2019

Conversation

ryandle
Copy link
Member

@ryandle ryandle commented Mar 27, 2019

#9924

-Add Include Symbols parameter
-Add Include Source parameter

Build and tests passing.

-Add Include Symbols parameter to pack command
-Add Include Source parameter to pack command

Build and tests passing.
@ryandle ryandle requested a review from sachinma as a code owner March 27, 2019 18:05
@sachinma sachinma requested a review from infin8x April 9, 2019 10:14
@sachinma sachinma added the Area: ArtifactsPackages Azure Artifacts Packaging Team label Apr 9, 2019
@sachinma
Copy link
Member

sachinma commented Apr 9, 2019

@alexmullans can you have someone in your team look at this

@infin8x
Copy link

infin8x commented Apr 15, 2019

@ryandle and I chatted on this offline. I'm happy to accept the Include Symbols and Include Sources parameters, but I'd like to avoid re-adding the Additional Arguments parameter. We'll be providing a better all-up story here late in Q2 or early in Q3 that allows you to specify whatever arguments you want, without causing hard-to-debug conflicts when Additional Arguments conflict with arugments provided by the task.

@ryandle, can you revise the PR to remove the Additional Arguments field?

@ryandle
Copy link
Member Author

ryandle commented Apr 18, 2019

Yes, working on a revision.

@ryandle
Copy link
Member Author

ryandle commented Apr 19, 2019

@alexmullans this PR is ready for a review from your team.

Copy link

@infin8x infin8x left a comment

Choose a reason for hiding this comment

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

These changes look good from PM, but I'll wait for a dev to provide the final 'Approve' review.

@zjrunner zjrunner self-assigned this Apr 19, 2019
@zjrunner zjrunner self-requested a review April 19, 2019 21:08
@zjrunner zjrunner removed their assignment Apr 19, 2019
Copy link
Member

@zjrunner zjrunner left a comment

Choose a reason for hiding this comment

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

was about to merge this, but saw it needs a version bump so it can be deployed and master is in M152 now.

Copy link
Contributor

@davidstaheli davidstaheli left a comment

Choose a reason for hiding this comment

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

Minor capitalization suggestion

Tasks/DotNetCoreCLIV2/task.json Outdated Show resolved Hide resolved
davidstaheli and others added 3 commits April 19, 2019 17:15
@zjrunner
Copy link
Member

Thanks for the contribution and the extra back and forth at the end!

@zjrunner zjrunner merged commit b889135 into microsoft:master Apr 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: ArtifactsPackages Azure Artifacts Packaging Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants