FrostyX / fedora-review-service

Fedora package reviews CI
7 stars 1 forks source link

Running after a bug has `fedora-review: +` #50

Open LecrisUT opened 3 months ago

LecrisUT commented 3 months ago

Currently it seems that after a bug has received the fedora-review: +, additional review requests are ignored. Sometimes the reviewee would want to further double-check the package builds before final upload.

E.g. when reviewing a dependency chain, the mock builds can be done locally by the reviewer and they could approve the whole chain early. But the review service is more thorough in the archs and distro versions that are tested, so it can be helpful to run one more review.

One UX idea is to reuse [fedora-review-service-build] to always run the review, but depending on where the filter for fedora-review is done, it can be rather tricky to implement.

FrostyX commented 3 months ago

One UX idea is to reuse [fedora-review-service-build] to always run the review, but depending on where the filter for fedora-review is done, it can be rather tricky to implement.

This sounds very reasonable to me

ppfeister commented 3 months ago

Experienced this myself and had the same thought.

Anyways, opening a PR.

ppfeister commented 3 months ago

Nevermind. My dev environment is being \~problematic\~, and I don't have the time to deal with that right now

Should theoretically be able to use bugzilla's get comments in a new func within rhbz.py to fetch latest

def get_comments(bug_id):
    bz = rhbz_client()
    return bz.getcomments(bug_id)

and edit L120-L123 in consumer.py to something like (with added get_comments import to L23)

if not (
    is_rhbz_ticket_open(bug)
    or ManualTrigger(get_comments(bug.id)[0]).recognized
    ):
        log.info("Not commenting on #%s, it is already closed "
                 "or has fedora-review+", bug.id)
        return

can't validate the syntax for get_comments/getcomments right now but something along those lines. Not sure if the indexing begins at the earliest for latest but can substitute for [-1] if backwards.


The + doesn't stop builds from occurring, it only stops the results from being posted. Since the action is triggered by a completed copr build rather than a bz message, the bugzilla comments need to be fetched every time.

FrostyX commented 2 months ago

Thank you for the patch @ppfeister, I will test it once https://pagure.io/fedora-infrastructure/issue/12054 gets resolved

ppfeister commented 2 months ago

Curious if it actually works as intended. Let me know how it goes whenever it gets tested