Zulip Chat Archive
Stream: general
Topic: Rant
Joachim Breitner (Apr 27 2022 at 10:56):
Hmm, quite annoying when I think my PR is ready for review, and I have labeled at that, and I’m patiently waiting for a review only to notice a few days later that CI wasn’t actually happy, and nobody is likely to look at it, and I only notice if I happen to check my open PRs. I wonder how best to avoid that? Maybe a bot that looks at PRs that are labeled awaiting-review
, but CI is red, and that comment & relabel the PR? Or maybe do that if nothing else is happening for a few days (no need to have bots annoy us when there is activity)?
Yaël Dillies (Apr 27 2022 at 10:57):
You can look at either of your Github branches and of your open PRs.
Eric Wieser (Apr 27 2022 at 11:01):
You can setup github to send you emails when CI fails; I think this might even be the default
Johan Commelin (Apr 27 2022 at 11:02):
@Joachim Breitner I'm sorry to hear that. I agree this can be annoying, because it's sometimes not clear where the responsibility for a PR lies, and so they can fall through the cracks.
I like your idea of a bot that draws some attention to such PRs.
Joachim Breitner (Apr 27 2022 at 11:04):
(oh, and I hope it’s not negative to call this “rant”, I’m not blaming anyone or such. It’s just nice to speakout what is bothering a bit.)
I think I might even get these emails, but they are also sent for PRs that I haven’t submitted for review, or for cancelled builds, so most of them are noise and that’s why I probably miss the relevant one.
Joachim Breitner (Apr 27 2022 at 11:07):
Indeed, clear responsibilities are probably a good idea. Assuming we only expect reviews on CI-green PRs by default (maybe there is a separate way to indicate “this doesn’t pass yet, and I know, but I still would like a review”), then that label plus a red CI is an invalid state that could be reconciled by a bot, by assigning to the user? But I’m worried it might cause too much noise…
Damiano Testa (Apr 27 2022 at 11:08):
There is also a slight difference between "red because mathlib did not build correctly" and "red but the olean's are available". I would say that flagging the first one is more important than the second one.
Arthur Paulino (Apr 27 2022 at 11:09):
If you look at #queue, it's just a fixed URL with some filtering parameters. I see two options:
- Creating a #broken_queue (or something along those lines) which points to such PRs
- You can create a URL of your own with red PRs labeled as
awating_review
and includeauthor:joachim
as a parameter so you can see your broken PRs from time to time
Joachim Breitner (Apr 27 2022 at 11:10):
I do look at my PRs from time to time, that’s how I find these stuck PRs right now. But I’m spoiled and lazy, and would like someone (better something) alert me when I made a mistake :-)
Last updated: Dec 20 2023 at 11:08 UTC