Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we suppress
git checkout
error code to avoid race, when remotest2-packages
branch wasn't created yet.So it works as a fall-back to default
master
branch if there is novX.Y
branch in packages.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought of that, but what bothers me is that we're implicitly treating master as default. There's a whole set of situations when mistake may cost us putting broken package to the repo. As I see it, if we don't have a sister branch, we fail, loud.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, but it's circular dependency:
st2
repo before you create branch inst2-packages
repost2-packages
repo before you create branch inst2
repoSo
master
is a fallback here.As I know current release machinery works this way:
vX.Y
branch inst2
repovX.Y
branch in dependent repos likest2web
(st2-packages
to add)With this order checkout branch which wasn't yet created in
st2-packages
repo will raise an error in Circle.^^ @manasdk is this order correct or no? (to make sure we have correct picture and if needed - I'll adjust the logic here).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, yes, it is a circular dependency and yes, there's no way to resolve it without failing or falling back to master. My point is that it's better to fail because failing has no side effects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still hoping someday we would just merge st2 and st2-packages and forget about this problem once and for all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^^
Yeah, initially, we couldn't think seriously about such option at all, because we couldn't touch entire existing
st2
codebase and release machinery with ourst2-packages
changes.So we kept that package logic isolated in new repo to avoid overflowing other team members (especially considering we had a lot of experiments and pretty big code flow).
When
st2-packages
matures and stabilizes, it's good to consider pros/cons again for keeping everything in one or separated repos 👍