This PR changes how our PR-commenter-bot-thing works.
Previously, it was a task running on pull-request events. This was causing multiple problems:
Even though the workflow was triggered by pull-request events, it was looking at the artifacts of the build events, which was a bit unusual.
Workflows triggered by pull-request events could finish before build events.
There might even be cases where a previous commits finishes after a later one, which complicates things a bit.
Overall, I did not have a great experience with things happening tied to the events emitted. So I decided to follow a more declarative approach; where we have a task that:
Looks at open PR's
Checks if the tests have passed (both of the PR head and base commit)
Checks if it already posted a comment about that commit.
Otherwise calculates the diff, and sends a comment.
It'd be easy to modify the script to update/delete existing comments.
And here are some disadvantages:
It always runs the code on master.
It is a scheduled task (every 10 minutes), because the account the bot runs at does not have permission to trigger new workflows at the end of pull-request or build tasks (that was my initial plan). This is to avoid triggering recursive workflows, GitHub says. But the script does not enter our nix-shell unless necessary, so the frequent runs should be fast (it has to install Nix tho, so it'd still take a few minutes)
I tried it until line 68 and had to fix a ton of annoying errors in the process. Then I made it a cronjob instead, and that requires me to get the workflow in master in order to try the rest. So it is likely to break a few times in master, and then we should try to fix it there.
This PR changes how our PR-commenter-bot-thing works.
Previously, it was a task running on
pull-request
events. This was causing multiple problems:pull-request
events, it was looking at the artifacts of thebuild
events, which was a bit unusual.pull-request
events could finish beforebuild
events.Overall, I did not have a great experience with things happening tied to the events emitted. So I decided to follow a more declarative approach; where we have a task that:
It'd be easy to modify the script to update/delete existing comments.
And here are some disadvantages:
pull-request
orbuild
tasks (that was my initial plan). This is to avoid triggering recursive workflows, GitHub says. But the script does not enter ournix-shell
unless necessary, so the frequent runs should be fast (it has to install Nix tho, so it'd still take a few minutes)I tried it until line 68 and had to fix a ton of annoying errors in the process. Then I made it a cronjob instead, and that requires me to get the workflow in
master
in order to try the rest. So it is likely to break a few times inmaster
, and then we should try to fix it there.