-
Notifications
You must be signed in to change notification settings - Fork 1
Prepare cli for the versioning feature #399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
| @@ -1 +1 @@ | |||
| v6.4.1 | |||
| 0 | |||
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.
Invalid version here, put the version in the swagger.yml
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.
We don't have version yet. I think we'll update it after meta versioning is merged but before CLI is merged ?
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.
Don't merge yet, I will check with gael
kerak19
left a comment
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.
Just to publish
fhacloid
left a comment
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.
Almost good.
internal/cyargs/stacks.go
Outdated
| cmd.Flags().String("stack-version-tag", "", "the stack version tag to use (e.g., v1.0.0)") | ||
| cmd.Flags().String("stack-version-branch", "", "the stack version branch to use (e.g., main)") | ||
| cmd.Flags().String("stack-version-commit-hash", "", "the stack version commit hash to use") |
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.
Make their name shorter, like:
--stack-tag
--stack-branch
--stack-commit-hash
I think shorter flags like:
--stack-tag -T
--stack-branch -B
--stack-commit-hash -H
Could work and not collide with other ones, try it, if it compiles it's good.
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 not a fan of these one letter flags, but the shorter versions look good
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
8ce720c to
c912207
Compare
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.
Pull request overview
Copilot reviewed 62 out of 63 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
c912207 to
9551c3b
Compare
|
The CI fails because CLI uses staging as a version of image. So it should pass after versioning is released on staging |
No description provided.