Add is primary environment to branch name contributor #236
Conversation
|
@bitwiseman mind taking a peek at this |
src/main/resources/jenkins/branch/BranchNameContributor/buildEnv.properties
Outdated
Show resolved
Hide resolved
I don't believe a JIRA does exist, and I am not sure that I can create one as I don't have an account and don't seem to be able to create one at the moment. |
Co-authored-by: Liam Newman <bitwiseman@gmail.com>
|
I created https://issues.jenkins.io/browse/JENKINS-64686 for this change |
|
@bitwiseman I have made the suggested changes, but the build is now failing on linux for the same reason as master currently. Could you take another look and possibly get this change merged? |
will be fixed by: #242 |
|
Thanks, @timja ! |
| if (branch.getAction(PrimaryInstanceMetadataAction.class) != null) { | ||
| envs.put("BRANCH_IS_PRIMARY", "true"); | ||
| } else { | ||
| envs.put("BRANCH_IS_PRIMARY", "false"); |
There was a problem hiding this comment.
Late review/question—instead of setting it to "false" (which is a truthy string), would it be possible to set it to the empty string?
| envs.put("BRANCH_IS_PRIMARY", "false"); | |
| envs.put("BRANCH_IS_PRIMARY", ""); |
This would make it easier for scripts to check this value without having to remember to convert it to a Boolean or comparing string literals. Please let me know if this makes sense, and I can open a PR.
(Hopefully it's not too late/disruptive to make this change since it looks like this hasn't been released yet. 🤞 )
There was a problem hiding this comment.
@usmonster
We haven't released since this was merged, so I'm open to changing the behavior but I'm not convinced we should. I generally think these variables being used by any number of systems, so designing specifically for shell script behavior isn't my goal. false is much clearer in my java-centric mind. Perhaps you could file a PR with the proposed change some examples and tag @cwholmes and we can discuss in a thread that isn't off the tail end of a merged PR. 😄
There was a problem hiding this comment.
Thanks for your quick response, @bitwiseman!
To be clear, this isn't a shell script-specific proposal, but I was thinking more about the most common context in which these variables will be used, i.e. in the Groovy scripts that define Jenkins pipelines. Maybe the variable shouldn't even be set if it's not a primary branch? In any case, I'll file a new PR to formally propose and discuss this. Thanks again!
There was a problem hiding this comment.
my 2 cents is you should only be comparing for the string == "true".
imo current behaviour is fine. but 🤷
https://issues.jenkins.io/browse/JENKINS-64686
'
During a pipeline execution, it is often valuable to know whether or not the branch currently being executed is the primary branch for the scm source.
This adds an environment contribution in the branch name contributor of
BRANCH_IS_PRIMARY.When the
PrimaryInstanceMetadataActionaction exists for the branch this environment variable is set to "true" otherwise it is set to "false".