Bh is pull request #117
Bh is pull request #117
Conversation
| @@ -107,6 +108,11 @@ function Get-BuildEnvironment { | |||
| PSModuleManifest = ${Build.ManifestPath} | |||
| ModulePath = ${Build.ModulePath} | |||
| } | |||
| if (${Build.Vars}.IsPullRequest) { | |||
| #This will ensure IsPullRequest is inserted into the ordered table after the BuildNumber property | |||
| $index = $($BuildHelpersVariables.keys).IndexOf("BuildNumber") | |||
There was a problem hiding this comment.
It's late so may be missing it, but any reason not to add IsPullRequest to the ordered hash definition above rather than this extra logic?
There was a problem hiding this comment.
My intention here (and answer to the rest of your questions sort of flow from this) is that since an Environment Variable can't be a boolean, just some text, that IsPullRequest would only exist if the run was a pull request, and so you could check just for existence of the environment variable. Here, I wanted to ensure that it would appear after BuildNumber in the ordered hash, but I couldn't just add it to the hash definition because it wouldn't always be defined. This addition here will add it into the ordered hash after BuildNumber, but only do so if there actually is a value
| 'GitHub Actions' {if ($ENV:GITHUB_EVENT_NAME -eq "pull_request") {"True"}; break} | ||
| #'Jenkins' {if ($Environment.CHANGE_ID) {"True"}} ???? is this correct? | ||
| #'Teamcity' {if ???????) {"True"}} who even knows | ||
| } |
There was a problem hiding this comment.
Two questions!
- Can we add a default case that sets this to
$null - Can we add else statements to indicate
$falsewhen a build system is identified, but no pull request evidence is found (i.e. not in a PR, unlike default case of not detecting being in a PR, which is $null) - Can we switch to boolean
$true/$falserather than strings?
Sorry to be a pain, and sorry for such a long delay!
There was a problem hiding this comment.
Doing anything differently with the variable here (setting it to null or false) would be a change of my intended functionality. Additionally, since these are going to be written to the environment as Environment variables, we'd lose the boolean-ness and they'd just end up as flat strings either way.
The intention was that the environment variable would only exist if it was a pull request, though I can see the benefit to knowing if you're definitively not in a PR build vs not being in a build environment where it can be detected.
There was a problem hiding this comment.
ok looked more closely at the code. The usual entry point is going to be Set-BuildEnvironment which takes the values from Get-BuildEnvironment which is what calls this (Get-BuildVariable).
In the end it runs this line:
$Output = New-Item -Path Env:\ -Name $prefixedVar -Value $BuildHelpersVariables[$VarName] -Force:$ForceIf the value is $true that will set the Environment Variable to be 'True', if it's $false it will set it as 'False', but if it's $null this will throw an error.
|
Hiii! Awesome idea! Sorry for the delay, and for the quantity of feedback! |
| BuildNumber = ${Build.Vars}.BuildNumber | ||
| ProjectName = ${Build.ProjectName} | ||
| PSModuleManifest = ${Build.ManifestPath} | ||
| ModulePath = $(Split-Path -Path ${Build.ManifestPath} -Parent) | ||
| } | ||
| if (${Build.Vars}.IsPullRequest) { | ||
| $BuildHelpersVariables.IsPullRequest = [bool]${Build.Vars}.IsPullRequest |
There was a problem hiding this comment.
Here at least we're setting PowerShell variables so it can be a boolean. Here it might make sense to set $false or $null, but then it's a slightly different behavior then when environment variables are used.
fixes #79
This reads environment variables in the following CIs to set a
IsPullRequestvariable if the current build has been triggered by a PR: AppVeyor, Gitlab CI, Azure Pipelines, Bamboo, GoCD, Travis CI, and Github Actions. I couldn't figure out a way to determine it in Jenkins or Teamcity but if there is a reliable way that someone is aware of it's easy to see in the code where to set it.I thought about figuring out way to determine this from Git directly but I'm not sure it's possible or even reasonable. GIt doesn't have a notion of a pull request directly it seems, in Github a pull request creates a branch something like
refs/remotes/pull/2/mergebut other systems may do something different so I'm not sure checking that the current branch matches a certain pattern is going to be reliableIf the build environment says the build was triggered by a PR, it says
IsPullRequestto beTrueotherwise it won't create the variable, so you can then check for the existence of the environment variable (or$BHIsPullRequestin the case of Set-BuildVariable.ps1)I also thought about setting some more details about the PR, like maybe the PR number, the source branch, the destination branch, etc, but I'd need help from people with experience in other CI systems to do that. I know where to find the values in my CI, Azure Pipelines, but less so in the others.
Finally, I'd added
CommitHasha while ago but realized I'd never added it toSet-BuildVariable.ps1so in addition to adding the logic forIsPullRequestin there I also addedCommitHashto that file.