Rename/remove old Marvin labels

Development
1

A few years ago, we added a bunch of labels for Marvin (RFC: marvin-mk2 the pull request bot). The bot was later disabled (Call for beta-testing reviewers · Issue #34 · timokau/marvin-mk2 · GitHub), but the labels remain.

I propose renaming these labels:

  • “awaiting_changes” = “2.status: needs-changes”
  • “needs_reviewer” = “2.status: needs-reviewer” (debatable usefulness, all PRs “need” a reviewer)
  • “needs_merger” = “2.status: needs-merger”

They are useful for tracking the current status and next steps for PRs. (Though arguably needs-merger could be replaced by the 12.approvals* labels.)

I propose removing:

  • “awaiting_reviewer” (just find another reviewer)
  • “awaiting_merger” (not really distinct from needs_merger)

What do you think?

1 Like
2

needs-changes makes sense, but I would check if an existing label fits first? and I think the others can be removed.

3

There is 2.status: blocked by pr/issue, but it only applies in certain cases.
Or 2.status: work-in-progress, I guess.

4

Yeah, I saw WIP used before, though for some reason it’s not used often by reviewers (mostly by the PR authors if at all). And it affects ofborg eval iirc.

5

Indeed: ofborg/ofborg/src/tasks/evaluate.rs at 5a4e743f192fb151915fcbe8789922fa401ecf48 · NixOS/ofborg · GitHub

If I understand the conditionals correctly, ofborg will still evaluate, but not build.

1 Like
6

I’d prefer if we added new labels rather than repurposing the Marvin labels.

Those should remain as is but perhaps renamed to make it clear they’re from Marvin. They could be removed at some point in the future when it’s clear that nobody cares to evaluate the Marvin data (e.g. when we have a replacement).

But generally having some sort of status labels to more accurately represent the status of a PR would be great.

2 Likes
7

I now added “2.status: needs-changes”, and put a warning in the description of the Marvin labels.

1 Like