Be able to configure conditional branches

[kemper]

resolves #172

[codecov-io]

Codecov Report

Merging #325 (f735b74) into master (30710d2) will increase coverage by 0.03%.
The diff coverage is 83.33%.

@@            Coverage Diff             @@
##           master     #325      +/-   ##
==========================================
+ Coverage   79.13%   79.17%   +0.03%     
==========================================
  Files          43       43              
  Lines        1107     1109       +2     
  Branches      262      262              
==========================================
+ Hits          876      878       +2     
  Misses         99       99              
  Partials      132      132              

Impacted Files	Coverage Δ
src/ConditionalApplyPlugin.groovy	68.75% <83.33%> (+4.46%)	⬆️

---

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 30710d2...f735b74. Read the comment docs.

[kemper]

I tried to follow the initial suggestion on issue #172 e.g. ConditionalApplyPlugin.withBranchApplyEnabledFor('qa').init() but I'm new to the terraform-pipeline project and didn't understand the intent of calling .init() at the end. I figured I'd take a stab at it and get feedback afterwards.

[kmanning]

I wanted to design the library in a way that allowed plugins to be enabled or disabled, as the user sees fit. I also wanted the enable/disable to be done as simply as possible - a static method fit the bill.

So init() is the convention that I came up with, to enable a plugin. Without calling init(), a plugin should not have any effect on your pipeline, with the exception of a handful of "default" plugins (which includes ConditionalApplyPlugin) See:

terraform-pipeline/src/TerraformEnvironmentStage.groovy

Line 7 in affa2f1

private static final DEFAULT_PLUGINS = [ new ConditionalApplyPlugin(), new ConfirmApplyPlugin(), new DefaultEnvironmentPlugin() ]

This is definitely an awkward implementation in hindsight. A better implementation could have treated ConditionalApplyPlugin (and every default plugin), like any other plugin, and had it implement an init() method. That init() method could then have been called in Jenkinsfile.init(), and achieved the same current behavior, without the code being any different from any other plugin.

[kmanning]

The implementation looks good. Can you update the CHANGELOG with your Issue? I'm also about to cut a release on master, and this change can go into v5.14, the next release.

[kmanning]

Hrm, I think I spent more time thinking/talking about the init method, that I didn't look into the implementation details enough. I think what you implemented is different from the intent outlined in the issue.

Your change will allow apply to run on all environments so long as the branch names match.

The intent described in the issue is that apply can run on a specific environment, regardless of what the branch is named.

Those are two different things. I pointed you in the direction of the existing Issue - is your UseCase different from what was outlined? If so, we should open a different issue.

[kmanning]

Per the discussion about the awkwardness of init and ConditionalApplyPlugin, I put in Issue #327

[kemper]

After chatting with @kmanning I realize I misinterpreted the goal of this issue. My PR does address half of issue #25 however, so I'm planning to update the PR for that issue.

[kmanning]

Can the Issue number and link be corrected? Both should point to Issue #25 - #25

[kmanning]

Other than the links in the ChangeLog, the changes look good.