Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend ci-link to PR title #4086

Open
ofirnk opened this issue Mar 11, 2021 · 16 comments
Open

Extend ci-link to PR title #4086

ofirnk opened this issue Mar 11, 2021 · 16 comments

Comments

@ofirnk
Copy link

ofirnk commented Mar 11, 2021

Similarly to repo CI status:
https://github.com/sindresorhus/refined-github/blob/main/source/features/ci-link.tsx
image

I propose adding a CI status next to PR name/title/number.
Reasons:

  1. On long PRs - you need to scroll A LOT to see CI status (for example )
  2. When you're in another tab - like changes - you need to switch tabs to know CI status. So you need to switch tab to conversations + scroll to bottom.
  3. When you're already scrolled in changes view - you need to lose context to know CI status. Like half way thru review, or direct link
@ofirnk
Copy link
Author

ofirnk commented Mar 11, 2021

I suggest to attach it to PR title - like that (expanded)
image

@ofirnk
Copy link
Author

ofirnk commented Mar 11, 2021

And like that (scrolled)
image

@kidonng
Copy link
Member

kidonng commented Mar 13, 2021

Yeah, I like how GitHub already does this on the mobile app:

Screenshot_20210313202643.png

@yakov116
Copy link
Member

For anyone that wants to do this see the feedback on #4100

@yakov116
Copy link
Member

I do not think this is possible anymore as GitHub stopped showing CI links on fork commit list see https://github.com/sindresorhus/refined-github/pull/4415/commits

@fregante
Copy link
Member

fregante commented May 31, 2021

Fantastic…

but maybe we can figure out their URL anyway if it still works

@kidonng
Copy link
Member

kidonng commented May 31, 2021

Maybe just an extra request:

  1. Get the latest commit if it's not available on the page (for example on Files tab)
  2. Fetch the commit page and extract CI icon from it

@yakov116
Copy link
Member

yakov116 commented May 31, 2021

2. Fetch the commit page and extract CI icon from it

What commit page are you referring to? Both in the PR and it the repo itself the ci-link is not there.

@kidonng
Copy link
Member

kidonng commented May 31, 2021

Something like this or this.

@yakov116
Copy link
Member

Ohh I see next to the title of the commit.

@fregante
Copy link
Member

fregante commented May 31, 2021

Maybe just an extra request:

Not ideal, especially since we already can't cache it. I'd rather just have it on the Conversation tab for the time being, and then maybe add a fetch if we have the commit information on the page or if have the direct URL to the icon.

@kidonng
Copy link
Member

kidonng commented Jun 1, 2021

I'd rather just have it on the Conversation tab for the time being, and then maybe add a fetch if we have the commit information on the page or if have the direct URL to the icon.

From what I can see, there are three situations (please correct if there's something wrong):

  • On Conversations tab, the latest commit along with the icon is already presented (not always though? Not sure). We don't need to make any request.
  • On Commits tab, we have the latest commit but not the icon, so we need one request to get the icon.
  • On Changes tab, we don't (?) have the latest commit info, so we need two requests, one for the commit list and one for the icon.

@ofirnk
Copy link
Author

ofirnk commented Jun 1, 2021

On changes we have the "view file" link which includes the latest commit hash.
On multiple commits I think there's "view changes at/between commits"

Latest commit from commit(s) tab --411aab..

image

Drop down in View File sub menu in changes tab

image

Link URL already in html - commit 411aab ...

image

@ofirnk
Copy link
Author

ofirnk commented Jun 1, 2021

@kidonng kidonng changed the title Show ci-link for PR next to it's title/number Extend ci-link to PR title Oct 31, 2021
@fregante
Copy link
Member

fregante commented Aug 5, 2022

This could be more easily implemented using:

Issues:

  • getPrInfo actually needs to also get the latest commit of the PR, not just the base commit
  • the latest commit can update multiple times while the tab is open, so we also need to replace the icon

But actually I suggest:

  • showing pr-ci-link only on the Conversation tab so we have access to the latest commit by looking on the page
  • listening to onNewComments to check whether we need to replace the icon if new commits appear

This would make it relatively easy(er) to implement.

@fregante
Copy link
Member

fregante commented Aug 5, 2022

But actually the solution in #4100 was pretty straightforward. Maybe it can be renewed using the latest adjustments to ci-link (the feature was rewritten in #5836)

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