-
Notifications
You must be signed in to change notification settings - Fork 9
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: run dependency sync script periodically and open a PR for it #339
Conversation
git commit -am "go.mod: sync dependencies with k6 core" | ||
git push -f -u origin "$BRANCH_NAME" | ||
|
||
cat <<EOF > pr-body.txt |
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 embedding texts in this way. I would prefer to have a script that generates this output.
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 sure I follow. WDYM by a script that generates the output? Something like this same code but in a separate .sh
file?
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.
Yes, exactly.
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.
What would be the advantage in this case? I agree the heredoc is not particularly pretty, but to me moving it to a separate script will make the yaml file harder to follow, as you would need to go somewhere else to check what the file is doing. Moreover such script wouldn't make sense outside of the context of this particular CI 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.
Basically, I think it is a bad practice to put any logic that does not depend on the CI environment (e.g. retrieving information provided by the CI) in the CI. Using a separate script allows running and testing this logic locally, if needed.
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.
Makes sense. I've added a depsync.sh
script that does pretty much everything that is not related to git. LMK if this looks better :)
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.
Even with the minor limitations mentioned in the PR, this is a great tool to prevent conflicts in the dependencies with k6. Great job.
Description
This PR introduces a CI job that runs
hack/depsync
every day, and creates (or updates if it exists) a PR with the changes it produces.PR should only be created if the job produces any changes to the git repository.
An example of the PR that would be created can be found here: roobre#28
Unfortunately, this does not currently trigger the regular PR workflow that would unittest those changes. This is a deliberate limitation of GH actions, as they automatically-generated
GITHUB_TOKEN
s they use do not trigger other workflows themselves.As a workaround for this, we can simply close and then reopen the PR, which will trigger the usual CI jobs for it, which the PR description says as a reminder. As I don't foresee having to do this very often, I think this workaround is enough and less maintenance than the alternative of creating a GitHub applciation.
Checklist:
make lint
) and all checks pass.make test
) and all tests pass.make integration-xxx
foragent
related changes)make e2e-xxx
fordisruptors
,kubernetes
orcluster
related changes)