-
Notifications
You must be signed in to change notification settings - Fork 222
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
Automatic update of the extension list #1731
Conversation
Automatic update of the k6 extension list in the docs/sources/next/extensions/explore.md file in case of changes based on the https://registry.k6.io/registry.json extension registry. The change is pushed to the extension-registry-changed branch and a pull request is opened (or updated) about the change.
In this temporary repo, you can see what kind of pull request the automation generates: |
I see that "npm run build" does not run because of src/templates/docs/explore-extensions.js. Am I right that since explore.md is not generated but edited manually, this file can be deleted? |
As a quick and dirty workaround, I kept a dummy extension in the extensions.json file for the build to run. Is that acceptable? |
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.
Thanks for putting this together. I've added some comments for the Bash script.
scripts/extension-registry-changed
Outdated
scriptdir=$(dirname $(readlink -f "$0")) | ||
docfile=$scriptdir/../docs/sources/next/extensions/explore.md | ||
|
||
tmpfile=/tmp/explore.$$ |
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 generally use mktemp
because it respects the TMPDIR
environment variable
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.
right, I'll fix it
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.
fixed
@jdbaldry, Thanks for the review, it's actually been quite a while since I last wrote scripts more seriously :) |
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.
One more thing that came to mind as I was thinking through the workflow execution is the failure modes of the script.
I generally put set -euf -o pipefail
at the start of the script to end execution for any failing command including those with piped output. The thought being that if any individual step fails, the rest of the script operates with false assumptions and then can only further mangle the final output.
It's not a huge concern because we always have the PR to review and reject if it has unexpected changes so up to you if you want to incorporate.
If you don't mind, I'll merge it, then modify it if necessary. |
What?
Automatic update of the k6 extension list in the
docs/sources/next/extensions/explore.md
file in case of changes based on the https://registry.k6.io/registry.json extension registry.
The change is pushed to the extension-registry-changed branch and a pull request is opened
(or updated) about the change.
The extension list is updated by the
scripts/extension-registry-changed
script.The script is started by the
extension-registry-changed
workflow defined in the.github/workflows/extension-registry-changed.yml
workflow file.The
extension-registry-changed
workflow is triggered by thegrafana/k6-extension-registry
repository when the registry changes using a
repository_dispatch
event of typeextension-registry-changed
.It is not a problem if the registry changes several times before merging the pull request,
the pull request will be updated. The "extension-registry-changed" branch can be deleted
after merging the pull request, it will be created again if necessary.
Checklist
npm start
command locally and verified that the changes look good.Related PR(s)/Issue(s)
#1640
Closes #1640