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

Users/pspill/dotnet2 #4520

Merged
merged 6 commits into from
Jun 14, 2017
Merged

Users/pspill/dotnet2 #4520

merged 6 commits into from
Jun 14, 2017

Conversation

pspill
Copy link

@pspill pspill commented Jun 12, 2017

This PR contains the following changes for the DotNetCoreCLI build task:

  1. enable dotnet restore to authenticate with VSTS and external feed sources
  2. add "dotnet nuget push" command, which can push to authenticated vsts and external feed sources
  3. add "dotnet pack" command, so users can complete the restore - pack - push scenario.

UI and experience is consistent with NuGetCommand build task for restore/pack/push

"q": "^1.4.1",
"archiver": "1.2.0"
"archiver": "1.2.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Check if we need OSS registration for this version of archiver.

Copy link
Author

Choose a reason for hiding this comment

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

downgraded to approved archiver 1.0.1


const dotnetPath = tl.which("dotnet", true);
for (const file of filesList) {
await dotnetPackAsync(dotnetPath, file, outputDir, nobuild, version, props, verbosity);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could/should we do this on parallel?

Copy link
Author

Choose a reason for hiding this comment

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

good idea, but I'm worried interleaved output on async tool.exec would be confusing on error,

}
}

function dotnetPackAsync(dotnetPath: string, packageFile: string, outputDir: string, nobuild: boolean, version: string, properties: string[], verbosity: string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: add function return type

Copy link
Author

Choose a reason for hiding this comment

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

fixed


for (const packageFile of filesList) {

await dotNetNuGetPushAsync(dotnetPath, packageFile, feedUri, apiKey, configFile, tempNuGetConfigDirectory);
Copy link
Contributor

Choose a reason for hiding this comment

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

same here, would we benefit from parallel push?

import * as commandHelper from "nuget-task-common/CommandHelper";


const NUGET_ORG_V3_URL: string = "https://api.nuget.org/v3/index.json";
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit. This should be in a common location.

Copy link
Author

Choose a reason for hiding this comment

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

fixed

tl.debug(`All URL prefixes: ${urlPrefixes}`);
}

let accessToken = auth.getSystemAccessToken();
Copy link
Contributor

Choose a reason for hiding this comment

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

Should all of the config code placed in a common location since it's the same as nugetRestore?

Copy link
Author

Choose a reason for hiding this comment

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

It's not exactly the same, so factoring out common areas cleanly would take some work.

},
"minimumAgentVersion": "2.0.0",
"instanceNameFormat": "dotnet $(command)",
"groups": [],
"groups": [
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit. indenting

@pspill pspill merged commit cc78b8e into microsoft:master Jun 14, 2017
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