-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
ci: add logql-analyzer CI #14495
ci: add logql-analyzer CI #14495
Conversation
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 believe that we need to address the comments before the merge.
.github/workflows/logql-analyzer.yml
Outdated
echo $PLUGIN_CONFIG_TEMPLATE > updater-config.yaml | ||
# replace placeholders with RELEASE_NAME and RELEASE TAG | ||
sed -i "s/\\"{{release}}\\"/\\"$RELEASE_NAME\\"/g" %s % configFileName | ||
sed -i "s/{{version}}/$RELEASE_TAG/g" %s % configFileName |
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 do not need this line because the config is created on the next step
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.
also, it's a JSONNET ;)
"update_jsonnet_attribute_configs": [ | ||
{ | ||
"file_path": "ksonnet/environments/logql-analyzer/supported-versions.libsonnet", | ||
"jsonnet_key": "${RELEASE_NAME}", |
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.
are ENV variables shared between the steps?
I see that Run shell command
step exports it but I am not sure if they are available on this step
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.
nope, good call, they either need to be echoed into $GITHUB_ENV
or set using the ::set-output
syntax, which I opted for
c5b3c6d
to
5e65bab
Compare
What this PR does / why we need it:
Adds
logql-analyzer
CI that got removed with the drone removal.Checklist
CONTRIBUTING.md
guide (required)feat
PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.docs/sources/setup/upgrade/_index.md
deprecated-config.yaml
anddeleted-config.yaml
files respectively in thetools/deprecated-config-checker
directory. Example PR