Add ability to update ci_update_branches from create-branch workflow #767
Conversation
| update_ci_branches: | ||
| description: "Behavior for updating ci_update_branches" | ||
| required: true | ||
| type: choice | ||
| default: "none" | ||
| options: | ||
| - "append-version" | ||
| - "replace-previous-version" | ||
| - "none" |
There was a problem hiding this comment.
Interesting idea, but i wonder if we should make this choice in the template-config. Giving the user choice usually leads to confusion.
I suspect we will hear: "Did you not select this option? We always do. Now I need to do it by hand."
There was a problem hiding this comment.
Like another variable to decide the behavior? I was also thinking of just having the default be different for pulpcore, have it be set to replace-previous-version instead of none since this is the behavior we've decided on for our release policy.
There was a problem hiding this comment.
I don't think pulpcore is special per se. Just as you said: Some plugins may not want to do it.
There was a problem hiding this comment.
What do you suggest then? Switch it to a template-config variable?
|
Let's make this a template_config variable. |
gerrod3
force-pushed
the
update-ci-branches
branch
2 times, most recently
from
89d58fb
to
4c66c94
Compare
| @@ -217,6 +218,28 @@ def main(): | |||
| """ | |||
| ), | |||
| ) | |||
| parser.add_argument( | |||
| "--add-version-ci-update-branches", | |||
There was a problem hiding this comment.
Not sure, the other one is actually more descriptive...
| "--add-version-ci-update-branches", | |
| "--add-ci-branch", |
So am i understanding correctly that this is another option like --github or --docs that specifies what parts of the plugin to change? I guess, it implies --github?
There was a problem hiding this comment.
The option only modifies the variable in the config file, you have to use --github in order to update the templated files.
There was a problem hiding this comment.
So yes in a way that it implies you also want to specify --github. XD
plugin-template
Outdated
| if args.replace_previous_version_ci_update_branches: | ||
| major_v, minor_v = new_version_branch.split(".", maxsplit=1) | ||
| minor_v = int(minor_v) - 1 | ||
| try: | ||
| index = config["ci_update_branches"].index(f"{major_v}.{minor_v}") | ||
| except ValueError: | ||
| config["ci_update_branches"].append(new_version_branch) | ||
| else: | ||
| config["ci_update_branches"][index] = new_version_branch | ||
| else: | ||
| config["ci_update_branches"].append(new_version_branch) |
There was a problem hiding this comment.
So we are assuming here, that a new major version will never replace the last version.
This sounds fine to me.
There was a problem hiding this comment.
I don't think I understand your statement. We will only replace the previous (V - 1) version from the list if --replace_previous_version_ci_update_branches is set. If we can't find that previous version then we will just append the new version to the end of the list.
There was a problem hiding this comment.
Replacing only works for 2.1 -> 2.2. If we released 2.1 -> 3.0 we would still keep 2.1 in the list. And that's probably fine because OTOH it's very rare, OTOH it's probably wise to support the last y of the previous x for a bit longer.
gerrod3
force-pushed
the
update-ci-branches
branch
2 times, most recently
from
b04c819
to
e8eeba5
Compare
gerrod3
force-pushed
the
update-ci-branches
branch
from
e8eeba5
to
7f9713d
Compare
Decided to add this functionality to the plugin-template script and have the create-branch workflow call with the new options if specified at dispatch time. Here is how the new dispatch screen looks:Plugins have different behavior from pulpcore, some don't even set this variable, so I added three different options to cover each scenario.
EDIT
Changed it to a new template_config variable:
ci_update_release_behavior.