(CODEMGMT-1350) Fall back to default branch even when overridden #1122
Conversation
|
This is an improvement requested in service of CD4PE, so I've used an internal ticket number. If this will mess with any community workflows, please let me know, and we can look into options for backwards compatibility. |
Magisus
force-pushed
the
default-branch-fallback
branch
2 times, most recently
from
1ef6fc7
to
0cb719b
Compare
| msg << "or resolve the default branch override '%{default_override}'," | ||
| vars[:default_override] = default_override | ||
| end | ||
|
|
||
| if default | ||
| msg << "or resolve default ref '%{default}'" | ||
| vars[:default] = default |
There was a problem hiding this comment.
I won't let me comment on the actual line, but do we think the "no default provided" message below will make sense if there's a default override but no actual default?
There was a problem hiding this comment.
The docs make clear that there is a "normal default" and a "default override". This is also the language the puppetfile uses for the options themselves.
There was a problem hiding this comment.
The whole message becomes: "Could not resolve control repo branch or resolve the default branch override 'override', and no default provided." I think it sounds okay.
| end | ||
|
|
||
| context "when default branch override is not resolvable" do | ||
| context "and default branch is provided" do |
There was a problem hiding this comment.
Should we test the case where the default branch is provided, but also not resolvable? That feels different from just not providing a default branch.
There was a problem hiding this comment.
We already test default suceeding (proving that we fall back), and elsewhere we test the default failing, demonstrating what that looks like. It feels like it's covered to me. But I can add a case if you think that's not sufficient.
There was a problem hiding this comment.
I'm not familiar with the tests in this repo, so if you think it's covered then I trust you.
I was just thinking that the default branch being unresolvable has different consequences depending on whether you use a default_override or not. When using a default_override, the default being unresolvable only matters if the default_override is also unresolvable.
There was a problem hiding this comment.
I added something for that last case. If the default is not resolvable and the override is, that doesn't matter because we never try it.
Magisus
force-pushed
the
default-branch-fallback
branch
from
0cb719b
to
678a823
Compare
doc/dynamic-environments/usage.mkd
Outdated
| @@ -67,10 +67,11 @@ Update a single environment and specify a default branch override: | |||
| r10k deploy environment my_working_environment --puppetfile --default-branch-override default_branch_override | |||
|
|
|||
| This will update the given environment and update all contained modules, overrideing | |||
There was a problem hiding this comment.
nit: can you change "overrideing" here since you're right next to it.
Currently, if a default branch override for a module is specified, if that branch doesn't exist, r10k will error without checking the module's normal default branch. This is not desirable, so this commit updates the fallback logic so that the default branch ref will also be checked, if the branch override fails to resolve. This makes it more likely that we will successfully resolve a module, rather than erroring.
Magisus
force-pushed
the
default-branch-fallback
branch
from
678a823
to
05fea8d
Compare
Currently, if a default branch override for a module is specified, if
that branch doesn't exist, r10k will error without checking the module's
normal default branch. This is not desirable, so this commit updates the
fallback logic so that the default branch ref will also be checked, if
the branch override fails to resolve. This makes it more likely that we
will successfully resolve a module, rather than erroring.
Please add all notable changes to the "Unreleased" section of the CHANGELOG in the format: