cake-build / cake

:cake: Cake (C# Make) is a cross platform build automation system.
https://cakebuild.net
MIT License
3.91k stars 730 forks source link

[Proposal] TFBuild.PullRequest API #2149

Open jnm2 opened 6 years ago

jnm2 commented 6 years ago

I wrote a build script today that had to check SYSTEM_PULLREQUEST_PULLREQUESTID to see if the current build was a pull request and to form a version string.

This seems like it might be a nice way to bring the TFBuild API to parity with the AppVeyor API. I'm willing to contribute the PR! 😃

Proposal

Source: https://docs.microsoft.com/vsts/build-release/concepts/definitions/build/variables#systempullrequestisfork

namespace Cake.Common.Build.TFBuild.Data
{
    public sealed class TFBuildEnvironmentInfo : TFInfo
    {
+       /// <summary>
+       /// Gets TF pull request info, or <see langword="null"/> if the current build
+       /// is not for a pull request.
+       /// </summary>
+       public TFBuildPullRequestInfo PullRequest { get; }
    }

+   public sealed class TFBuildPullRequestInfo : TFInfo
+   {
+       public bool IsFork => GetEnvironmentBool("SYSTEM_PULLREQUEST_ISFORK");
+
+       public int Id => GetEnvironmentInteger("SYSTEM_PULLREQUEST_PULLREQUESTID");
+
+       public int Number => GetEnvironmentInteger("SYSTEM_PULLREQUEST_PULLREQUESTNUMBER");
+
+       public string SourceBranch => GetEnvironmentInteger("SYSTEM_PULLREQUEST_SOURCEBRANCH");
+
+       public string SourceRepositoryUri => GetEnvironmentInteger("SYSTEM_PULLREQUEST_SOURCEREPOSITORYURI");
+
+       public string TargetBranch => GetEnvironmentInteger("SYSTEM_PULLREQUEST_TARGETBRANCH");
+   }
}

Questions

  1. You have IsPullRequest for AppVeyor. Which is preferable?

    • TFBuild.Environment.PullRequest == null
    • TFBuild.Environment.IsPullRequest
    • TFBuild.Environment.PullRequest.IsPullRequest
  2. To decide whether PullRequest == null or IsPullRequest should be true, we could check whether BUILD_REASON is PullRequest. However, if possible, it might be most flexible to check for the presence or absence of SYSTEM_PULLREQUEST_PULLREQUESTID.

devlead commented 6 years ago

Sure sounds like a good addition

  1. IsPullRequest
  2. Unsure, SYSTEM_PULLREQUEST_PULLREQUESTID feels more correct, but not read docs, what do they say?