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.
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
Geodesic v4 #961
base: main
Are you sure you want to change the base?
Geodesic v4 #961
Changes from 8 commits
0da2146
3dae18b
85a32de
9f454e7
a39ac0e
c55309e
60f537a
5d771d8
2c7f7c5
b2d1b45
94e2add
d9b0786
4cdafb3
fd9e32f
12e6954
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
🧹 Nitpick (assertive)
Consider simplifying the latest tag conditions.
The conditions for enabling latest tags are complex and repeated. Consider extracting the condition to a composite expression.
📝 Committable suggestion
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.
That doesn't work
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.
@Nuru, understood. Could you clarify what specifically isn’t working about the environment variable approach? If you can share any error messages or logs, that might help pinpoint the issue and figure out a better alternative. Otherwise, we can leave the existing logic in place. Let me know how you’d like to proceed.
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.
You are attempting to set an environment variable in the
with:
section of an action, which doesn't work. AFAIK setting it in theenv:
would not make it available in thewith:
section either.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.
@Nuru, indeed, GitHub Actions doesn't allow setting an environment variable in
env:
and then referencing it in thewith:
section. An alternative approach is to produce an output in one step (e.g., by echoing to$GITHUB_OUTPUT
) and then reference that output in thewith:
section. For example:This way, you can reuse the condition across subsequent steps without having to rely on environment variables that do not get expanded in the
with:
block.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.
Fix kubectl version - current version appears incorrect
The specified kubectl version 1.30 is not valid as the latest stable version is 1.28.
Apply this diff to fix the version:
📝 Committable suggestion
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.
No, 1.31 is the current stable version, and 1.32.0 is out
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.
@Nuru Thank you for clarifying. Understood that you want to keep using a more recent version (1.31 or 1.32). Since you’ve decided against reverting to 1.28, we’ll leave things as-is or update to one of those newer versions as you prefer.
(┬┬﹏┬┬)
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.
🧹 Nitpick (assertive)
Fix the broken link to release notes.
The link to the release notes is using a relative path that might not work on GitHub. Consider using the full GitHub URL.
📝 Committable suggestion