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

Add PR titles to UI #224

Merged
merged 6 commits into from Nov 30, 2021
Merged

Add PR titles to UI #224

merged 6 commits into from Nov 30, 2021

Conversation

gs0510
Copy link
Collaborator

@gs0510 gs0510 commented Nov 29, 2021

Ocurrent fetches all the refs and PRs using the github graphql interface. The interface is abstracted out so we can get the title
making another graphql query to github. It is not possible to just make one query because all the types are not exposed by OCurrent, so we would need to do a lot of copying of types if we wanted to avoid making another query. The frontend then displays the title if it's available in the benchmark_metadata table.

Ocurrent fetches all the refs and PRs using the github graphql
interface. The interface is abstracted out so we can get the title
making another graphql query to github. It is not possible to just
make one query because all the types are not exposed by OCurrent,
so we would need to do a lot of copying of types if we wanted to
avoid making another query.
@gs0510
Copy link
Collaborator Author

gs0510 commented Nov 29, 2021

Screenshot from 2021-11-29 17-37-08
)

@gs0510
Copy link
Collaborator Author

gs0510 commented Nov 29, 2021

Screenshot from 2021-11-29 17-37-20

@art-w
Copy link
Contributor

art-w commented Nov 29, 2021

Very nice! As a next step, this PR will allow us to update the sidebar as the preview proposed in #126 was looking really good!

I think the current code doesn't update the missing PR titles for already processed commits (because the pr_title only gets inserted during setup_metadata if the exists test failed).

@gs0510
Copy link
Collaborator Author

gs0510 commented Nov 29, 2021

Ah nice! I will edit the frontend to look like what's proposed in #126 (it looks nicer imo), maybe we don't care about the sidebar, it doesn't look like it'd be very useful, what are your thoughts @art-w ? :)

@art-w
Copy link
Contributor

art-w commented Nov 29, 2021

I think it would be nice to have the PR titles in the sidebar, as they would help navigation :)

@gs0510
Copy link
Collaborator Author

gs0510 commented Nov 30, 2021

Screenshot from 2021-11-30 18-11-51
You can now see the pr title on the sidebar!

@art-w
Copy link
Contributor

art-w commented Nov 30, 2021

LGTM! Thanks!

@art-w art-w merged commit cf4a7b8 into ocurrent:main Nov 30, 2021
@gs0510 gs0510 mentioned this pull request Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants