#4835 Request for clarification: commit vs PR flags
Closed: Fixed 3 years ago by pingou. Opened 3 years ago by csomh.

While working in integrating Packit-as-a-Service with
git.centos.org, which as of today runs Pagure 5.8.1, we hit the issues bellow.
Would be nice to discuss them, to figure out if there is anything that we are
missing and that could improve our understanding of flags in Pagure, or if
Pagure's way of flagging commits/PRs could be improved.

Packit Service uses commit flags to communicate the status of builds to users.
In GitHub we set these flags on the last (head) commit of PRs. These then show
up on the conversation tab in PRs, and as green tick-marks in the PR's Commits
tab and next to the commit subject line when viewing the commit itself. This
behaviour gives users an overview of the current status of the PR on the
conversation tab, but it's also very explicit about which commit in the PR was
checked and what the outcome of that check was.

We were expecting a similar behaviour in Pagure, but then found out, that
although one can set the flags on the last commit from the PR, there will be
no sign of this neither on the PR's Comments tab, nor the Commits tab, nor
when visiting the commit page (by clicking the commit hash on the Commits
tab).

Then we read the docs (yeah, I know: duh!), and found out that Pagure has a
separate API for setting flags on PRs and using this API endpoint will cause
the flags showing up on the Comments tab.

We also found out that the commit flags we were original setting do show up in
Pagure Web UI, but one has to visit the commit page in the destination repo
(/{namespace}/{repo}/c/{commit_hash}). When clicking the commit hash in the
PR's Commits tab though, the commit page is displayed in the origin repo
(/{fork}/{username}/{namespace}/{repo}/c/{commit_hash}).

The thing that confuses us in the above behaviour, is that we see no clear way
indicating to our users which commit the PR status is referring to, so users
might not know whether the status displayed on the PR's discussion page is for
the last commit of the PR or some previous one.

I've seen on Pagure PRs, that CI
there mentions the commit SHA in the comment part of the PR flags. This
could also work for Packit, but I find this as a not good enough user experience:
users will need to match the commit hash from the comment with a commit from
the commit list in order to decide whether the status displayed is up to date
or still relevant.

Sorry for the above being very fuzzy, I'm really not sure if this should be a
feature request or an improvement in our understanding of the Pagure API. Some
of the questions I would like to answer:

  • Is it possible to match the user experience of GitHub when it comes to flags
    with the Pagure API/web UI?
  • What is the intention behind separating commit and PR flags in Pagure?

Metadata Update from @ngompa:
- Issue set to the milestone: 5.11

3 years ago

Metadata Update from @ngompa:
- Issue set to the milestone: 5.12 (was: 5.11)

3 years ago

Metadata Update from @pingou:
- Issue assigned to pingou

3 years ago

The PR https://pagure.io/pagure/pull-request/5027 changes the behavior of the PR flag endpoint to actually flag the commit at the top of the PR instead.

Meaning if there will now be two ways to flag a commit, either the PR flag endpoint of the commit flag endpoint.

The PR has an example of what the new page will look like. I've kept the "Flags" section on the right hand side so we do not loose the information about the current flags. In the future, we should drop that section, same things for the flags shown on the PR list page.

Thank you @pingou !

So just to check: When we use the commit-flag, it will be shown in the new place as you described in the linked PR.

We would probably not use the pr-flag because people can push in the meantime and we will pick the wrong commit. So we would like to explicitly pick the commit we are working on.

So just to check: When we use the commit-flag, it will be shown in the new place as you described in the linked PR.

Correct

We would probably not use the pr-flag because people can push in the meantime and we will pick the wrong commit. So we would like to explicitly pick the commit we are working on.

That's a risk indeed.

Commit 622764a fixes this issue

Login to comment on this ticket.

Metadata
Related Pull Requests
  • #5027 Merged 3 years ago