Skip to content

.github/workflows: use ofborg-eval context for pending status#195984

Merged
zowoq merged 1 commit intoNixOS:masterfrom
ncfavier:ofborg-pending
Oct 14, 2022
Merged

.github/workflows: use ofborg-eval context for pending status#195984
zowoq merged 1 commit intoNixOS:masterfrom
ncfavier:ofborg-pending

Conversation

@ncfavier
Copy link
Member

@ncfavier ncfavier commented Oct 14, 2022

What we want

If I understand correctly, we want new(ly updated) PRs to get a "Waiting for OfBorg" pending status, so that they stay pending at least until OfBorg is summoned.

What is currently happening

When a PR is created, the pending-set.yml workflow creates a pending status with context Waiting for ofborg and description "This pending status will be cleared when ofborg starts eval.".

However, the pending-clear.yml workflow only runs when the OfBorg check suite becomes completed, which happens when all of the check runs (not statuses) created by OfBorg are completed (you can see the check runs posted by OfBorg in the "Checks" tab of a PR, under the "OfBorg" drop-down). This only happens once OfBorg finishes evaluation, because it then posts the "Evaluation Performance Report" check run, but it can happen again e.g. when all queued packages are done building.

As far as I can tell, the ofborg-eval-started status introduced in NixOS/ofborg#527 does not change anything (since it is a status and not a check) and can be removed (independently from this change).

Proposed fix

Instead of adding a pending status with context Wait for ofborg, make the context ofborg-eval and the description "Wait for OfBorg...". That way, the status will be reused by OfBorg when it starts evaluation and we don't need to clear it any more.

Will this work?

I think so. I tested this on my fork. After creating the PR, the pending status was set as expected:

screenshot-2022-10-14-144523

Then, after simulating OfBorg by running

curl -H "Authorization: Bearer $MY_TOKEN" \
    -d '{"context": "ofborg-eval", "state": "success", "description": "^.^"}' \
    "https://api.github.com/repos/ncfavier/nixpkgs/commits/$SHA/statuses"

the status was correctly overwritten:

screenshot-2022-10-14-145104

Instead of adding a pending status with context `Wait for ofborg`, make
the context `ofborg-eval` and the description "Wait for OfBorg...". That
way, the status will be reused by OfBorg when it starts evaluation and
we don't need to clear it any more.
@ncfavier ncfavier requested review from a team, Mic92 and zowoq as code owners October 14, 2022 14:05
@github-actions github-actions bot added the 6.topic: policy discussion Discuss policies to work in and around Nixpkgs label Oct 14, 2022
@ncfavier ncfavier requested review from cole-h and grahamc October 14, 2022 14:06
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Oct 14, 2022
@zowoq
Copy link
Contributor

zowoq commented Oct 14, 2022

Let's see if it works.

@zowoq zowoq merged commit 9b480c2 into NixOS:master Oct 14, 2022
@ncfavier ncfavier deleted the ofborg-pending branch October 15, 2022 07:40
@ncfavier
Copy link
Member Author

One small problem I did not anticipate: ofborg is so fast that there's a race condition where the "Waiting for OfBorg" message is written after it starts. Not too bad since it will be overwritten again soon, but still annoying.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: policy discussion Discuss policies to work in and around Nixpkgs 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants