Fixes #2259#2909
Conversation
alexr00
left a comment
There was a problem hiding this comment.
Just one change needed I think! I still think we should also offer to "move to"/"rename" the old branch if it exists, but that can be a separate PR since this is a good change regardless.
| this._branchName = branchNameConfig; | ||
| } | ||
| } | ||
| if (!this._branchName) { |
There was a problem hiding this comment.
We also need to do this if if this._branchName changed in the previous block since it includes branch name validation.
There was a problem hiding this comment.
@burkeholland this is the only change I see that is needed before merging!
There was a problem hiding this comment.
Sorry - I'm not following the logic here. Are you referring to the branchNameConfig section? I don't know why we need to check for the branch existing there since all that does is compose the branch name from the config.
There was a problem hiding this comment.
I was thinking something like this, since we still needed to validate the branch name:
There was a problem hiding this comment.
I have pushed that change though, so I will merge this PR for the September milestone after we release.
alexr00
left a comment
There was a problem hiding this comment.
I pushed the change, it all looks good. Thank you!
Check to see if new branch name per config is different from what is in state. If so, and branch has been deleted, update branch name to new config calculated value.