Skip to content

Fix run with Azure Pipelines as CI Source and Azure Repos Git as Request Source#1284

Merged
orta merged 14 commits into
danger:masterfrom
damien-danglard:master
Jan 19, 2021
Merged

Fix run with Azure Pipelines as CI Source and Azure Repos Git as Request Source#1284
orta merged 14 commits into
danger:masterfrom
damien-danglard:master

Conversation

@damien-danglard

@damien-danglard damien-danglard commented Jan 11, 2021

Copy link
Copy Markdown
Contributor

When running Danger with Azure Pipeline (CI Source), 2 CI Source (Danger::CI subclass) match : Danger::AzurePipelines and Danger::VSTS, the first is always selected.
The problem is, when I use Azure Repos Git (Request Source), Danger::RequestSources::VSTS is in Danger::VSTS.supported_request_sources but not in Danger::AzurePipeline.supported_request_sources.
So, like it's Danger::AzurePipelines selected as default CI Source, an error is throw.

I added a filter in Danger::AzurePipelines.validates_as_ci? to return false if Request Source is Azure Repos Git without changing the previous value in other case.
For that I use BUILD_REPOSITORY_PROVIDER environment variable defined by Azure Pipeline (see Predefined variables - Azure Pipelines). The value of this environment variable is "TfsGit" only if Request Provider (Request Source) is Azure Repos Git.

Conclusion: If I use Azure Pipelines with Azure Repos Git, Danger::AzurePipelines will not be selected because it does not support Danger::RequestSources::VSTS. The selected CI Source will be Danger::VSTS, as expected.

PS: I think Danger::AzurePipelines and Danger::VSTS should be merged because I think they represent the same CI Source (Azure Pipeline is the evolution of VSTS).

@danger-public

Copy link
Copy Markdown
1 Warning
⚠️ Tests were not updated

Generated by 🚫 Danger

@orta

orta commented Jan 11, 2021

Copy link
Copy Markdown
Member

Yeah, I think VSTS and Pipelines are now basically the same - it'd be good to get a test for this, but otherwise looks good to me

@damien-danglard

Copy link
Copy Markdown
Contributor Author

I added tests for Danger::AzurePipelines but I think CI build should be rerun.

@orta orta merged commit 9a27312 into danger:master Jan 19, 2021
@orta

orta commented Jan 19, 2021

Copy link
Copy Markdown
Member

Thanks! Looks good to me

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