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 Bitbucket Cloud provider #789

Merged
merged 4 commits into from Jul 30, 2022
Merged

Add Bitbucket Cloud provider #789

merged 4 commits into from Jul 30, 2022

Conversation

LoremFooBar
Copy link
Contributor

I know no one asked for it, but seemed pretty straightforward to add support for Bitbucket Cloud so I did.

@dnfadmin
Copy link

dnfadmin commented Jul 27, 2022

CLA assistant check
All CLA requirements met.

@codecov
Copy link

codecov bot commented Jul 27, 2022

Codecov Report

Merging #789 (1d25d20) into main (032cbc6) will increase coverage by 0.04%.
The diff coverage is 40.00%.

@@            Coverage Diff             @@
##             main     #789      +/-   ##
==========================================
+ Coverage   81.33%   81.38%   +0.04%     
==========================================
  Files          72       73       +1     
  Lines        3820     3830      +10     
==========================================
+ Hits         3107     3117      +10     
  Misses        713      713              
Impacted Files Coverage Δ
...GitVersioning/CloudBuildServices/BitbucketCloud.cs 33.33% <33.33%> (ø)
src/NerdBank.GitVersioning/CloudBuild.cs 78.94% <100.00%> (+1.16%) ⬆️
...ank.GitVersioning/ManagedGit/MemoryMappedStream.cs 68.75% <0.00%> (+12.50%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 032cbc6...1d25d20. Read the comment docs.

Copy link
Collaborator

@AArnott AArnott left a comment

Choose a reason for hiding this comment

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

Fantastic. Thank you.

@AArnott AArnott added this to the v3.6 milestone Jul 28, 2022
@AArnott
Copy link
Collaborator

AArnott commented Jul 28, 2022

The failure in AzP looks unrelated to your change. I've queued a fresh build of the main branch to confirm whether your PR is somehow causing this to fail.

public class BitbucketCloud : ICloudBuild
{
/// <inheritdoc />
public bool IsApplicable => !string.IsNullOrWhiteSpace(Environment.GetEnvironmentVariable("BITBUCKET_REPO_SLUG"));

Choose a reason for hiding this comment

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

There is a risk of a false positive, if BITBUCKET_REPO_SLUG is defined by a Jenkins pipeline using Bitbucket Server, for example. Because Atlassian uses the "repository slug" term in REST API documentation, a pipeline author might use that name. Some other variable from https://support.atlassian.com/bitbucket-cloud/docs/variables-and-secrets/ might have a lower risk of such a conflict. BITBUCKET_PIPELINE_UUID looks most suitable, as Bitbucket Data Center and Server don't have pipelines, and Jenkins pipelines don't have UUIDs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see your point. If we really want to be on the safe side, I could test for multiple environment variables that are not commonly used:
BITBUCKET_PIPELINE_UUID
BITBUCKET_STEP_UUID
BITBUCKET_STEP_TRIGGERER_UUID

The chance of anyone defining all of these variables outside of an actual Bitbucket Pipelines environment is incredibly low.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you for calling out this risk, @KalleOlaviNiemitalo

@LoremFooBar
Copy link
Contributor Author

The failure in AzP looks unrelated to your change. I've queued a fresh build of the main branch to confirm whether your PR is somehow causing this to fail.

I see the pipeline for main passed. Looking at the logs I am not sure what exactly failed, is there anything I can try locally? If you think this is a transient error, I will eventually push a fix for @KalleOlaviNiemitalo's comment so it will trigger another run that will hopefully pass.

@LoremFooBar LoremFooBar marked this pull request as ready for review July 28, 2022 16:27
@AArnott
Copy link
Collaborator

AArnott commented Jul 28, 2022

I do think this is a transient error. It appears my request to rebuild your PR got passed the prior failing point. I see you marked this PR as ready to merge, but are you planning to address the env var risk that's in the active thread?

@LoremFooBar
Copy link
Contributor Author

@AArnott sorry, missed a failed push. But seems we have the same pipeline issue again...

@AArnott AArnott enabled auto-merge (squash) July 30, 2022 02:50
@AArnott AArnott merged commit 0568520 into dotnet:main Jul 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants