Skip to content
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

Pre-commit update: Add ref for scheduled build and hint for maintainers #32

Merged
merged 4 commits into from
Mar 12, 2024

Conversation

christophfroehlich
Copy link
Contributor

CI pipeline is not triggered for github-actions bot with GITHUB_TOKEN

When you use the repository's GITHUB_TOKEN to perform tasks, events triggered by the GITHUB_TOKEN, with the exception of workflow_dispatch and repository_dispatch, will not create a new workflow run.

It can happen now that pre-commit fails after an update, e.g., see here.
I don't know how to fix this, we probably would need to create separate tokens for this workflow?

Maintainers can manually run the work due to the workflow-dispatch trigger on the newly created PR, but this won't be listed in the PR. I propose making the PR draft and add a hint for the maintainers to do so.

Furthermore, I added a ref_for_scheduled_build input to make the workflow configurable for other branches than master too.

@fmauch
Copy link
Contributor

fmauch commented Mar 11, 2024

I think I don't quite get the problem mentioned above. Sure things can break if the version is updated, this is why we run pre-commit also in the PR updating the hooks, right? ros-controls/ros2_control_demos#462 shows a failed pre-commit run in the checks. I guess at that point we should just not just merge it, but address upcoming problems exactly as you did in ros-controls/ros2_controllers#1073. What am I missing here?

+1 for using the ref for scheduled build.

@christophfroehlich
Copy link
Contributor Author

The event is not triggered by default (see the github docs above), but only if something changes in the PR: In your linked PR I rebased it on top of the master, this is why the workflow got started. See this PR as an example where it didn't. Bence merged it without noticing that the pre-commit will fail in future. btw: Setting it draft and reopening it does not trigger the workflow.

Copy link
Contributor

@fmauch fmauch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation. Given the circumstances this looks good!

@christophfroehlich christophfroehlich merged commit 7c65592 into master Mar 12, 2024
2 checks passed
@christophfroehlich christophfroehlich deleted the add_ref_for_scheduled_build branch March 12, 2024 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants