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

[PipelineArtifactPluginV2] Add support of minimumBuildStatus property #2560

Merged

Conversation

Thilas
Copy link
Contributor

@Thilas Thilas commented Oct 30, 2019

@jahsu-MSFT, @johnterickson, as discussed in microsoft/azure-pipelines-tasks#11556, here are my changes that add a minimumBuildStatus property to the PipelineArtifactPluginV2 task plugin.

@jahsu-MSFT
Copy link
Contributor

Thanks @Thilas . The agent change looks good to me. Please re-sync with master and fix the conflicts. cc @johnterickson

@@ -177,15 +190,15 @@ public class DownloadPipelineArtifactTaskV2_0_0 : PipelineArtifactTaskPluginBase
{
if (pipelineVersionToDownload == pipelineVersionToDownloadLatest)
{
pipelineId = await this.GetPipelineIdAsync(context, pipelineDefinition, pipelineVersionToDownload, projectName, tagsInput);
Copy link
Contributor

@jahsu-MSFT jahsu-MSFT Nov 4, 2019

Choose a reason for hiding this comment

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

We can pass the resultFilter to the function and set default to Succeed so that we don't need to always pass two bool variables here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

# Conflicts:
#	src/Agent.Plugins/PipelineArtifact/PipelineArtifactPluginV2.cs
@Thilas
Copy link
Contributor Author

Thilas commented Nov 4, 2019

All done, I let you finalize the review. Thanks.

@jahsu-MSFT
Copy link
Contributor

Looks good to me. I'm approving the code changes in the agent first since we need to wait until the agent is released and then release the task. @jtpetty Can you please take a look and merge the PR. Thanks,

Copy link
Contributor

@jtpetty jtpetty left a comment

Choose a reason for hiding this comment

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

LGTM

@jtpetty jtpetty merged commit 369492e into microsoft:master Nov 5, 2019
@Thilas
Copy link
Contributor Author

Thilas commented Nov 5, 2019

Thank you!

@Thilas Thilas deleted the download-artifacts-from-failed-builds branch November 5, 2019 17:26
jtpetty pushed a commit that referenced this pull request Dec 18, 2019
jtpetty pushed a commit that referenced this pull request Dec 18, 2019
* Cherry picking #2560 to m163

* bump version
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.

3 participants