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

Meta: post PR comment or status with a link to ecma262-compare #1891

Open
arai-a opened this issue Mar 7, 2020 · 11 comments
Open

Meta: post PR comment or status with a link to ecma262-compare #1891

arai-a opened this issue Mar 7, 2020 · 11 comments
Labels

Comments

@arai-a
Copy link
Contributor

arai-a commented Mar 7, 2020

Hi, I'm developing a tool for comparing the output HTML of the spec revisions and PRs

It can compare single revision, or multiple revisions,
the original purpose is to help updating implementation

Also it can compare PR change,
I hope this helps reviewing process

Those data is updated periodically (every 30 minutes for now)

Then, as part of it, I'm experimenting posting a comment to PR with the URL for it.
(currently posting comments to arai-a/workflow-test#2, instead of each PR here)
and I want to integrate it to this repository.
(tracked in arai-a/ecma262-compare#1 and arai-a/ecma262-compare#16)

Any feedback is welcome! :)

@arai-a
Copy link
Contributor Author

arai-a commented Mar 7, 2020

Also, it contains the snapshot of each revision/PR.
"Snapshot" links in

I think this part is related to #1883

@devsnek
Copy link
Member

devsnek commented Mar 7, 2020

this is amazing and we need to integrate it

@devsnek
Copy link
Member

devsnek commented Mar 7, 2020

would it be possible for the pr links to go to a page without the large header ui? we don't need to be able to switch to different prs/revisions/etc in that context (just between the commits within the pr)

@arai-a
Copy link
Contributor Author

arai-a commented Mar 7, 2020

would it be possible for the pr links to go to a page without the large header ui? we don't need to be able to switch to different prs/revisions/etc in that context

okay, I'll make the header collapse-able, and controllable with query string.

(just between the commits within the pr)

Currently, only the head commit data is created, so not possible to switch between commits within PR.
Previously, it was creating all commits, but sometimes PR can contain 10+ commits for addressing review comments, and it takes much time (5 min per single commit) and space (6MB HTML + 6MB JSON per single commit) to handle all of them

@arai-a
Copy link
Contributor Author

arai-a commented Mar 7, 2020

Here's the link with the header collapsed.
https://arai-a.github.io/ecma262-compare/?pr=1597&collapsed=1

@ljharb
Copy link
Member

ljharb commented Mar 7, 2020

We’d want to find a way to host it ourselves, i think; but forking or transferring it to the tc39 org seems like it’d handle that.

For the PR status - to start, let’s match the deploy previews and make it a “details” link; when we have a commenting solution for the deploy previews, we’ll want to combine them (ie, we probably won’t want N comments, one for each info link).

@ljharb ljharb added the meta label Mar 7, 2020
@arai-a
Copy link
Contributor Author

arai-a commented Mar 7, 2020

We’d want to find a way to host it ourselves, i think; but forking or transferring it to the tc39 org seems like it’d handle that.

by "host", do you mean running it on standalone server/website?
anyway, I'm fine transferring the repository here.

For the PR status - to start, let’s match the deploy previews and make it a “details” link; when we have a commenting solution for the deploy previews, we’ll want to combine them (ie, we probably won’t want N comments, one for each info link).

okay, I'll look into checks API.
https://developer.github.com/v3/checks/

@arai-a
Copy link
Contributor Author

arai-a commented Mar 7, 2020

With checks API without GitHub App (so, authenticated with GitHub action's token), "details" link leads to a page inside GitHub, not a specified URL.
example: https://github.com/arai-a/workflow-test/pull/2/checks?check_run_id=492575147
I can put URL there to link to ecma262-compare page, but it requires extra click than other existing checks.
(I can put URL in the title, and it's shown in PR page arai-a/workflow-test#2 , but it's not linkified)

to use the same way as other checks, I think I have to create GitHub App, with using some other services (maybe Heroku?)

@ljharb
Copy link
Member

ljharb commented Mar 7, 2020

Hm, with the right token we should be able to set the PR status link to whatever we want, but maybe checks works differently.

@arai-a
Copy link
Contributor Author

arai-a commented Mar 7, 2020

indeed, statuses API works :)
https://developer.github.com/v3/repos/statuses/#create-a-status

I'll rewrite it to use statuses API

@arai-a arai-a changed the title Meta: post PR comment with a link to ecma262-compare Meta: post PR comment or status with a link to ecma262-compare Mar 9, 2020
@arai-a
Copy link
Contributor Author

arai-a commented Mar 10, 2020

Okay, looks like it's working now.
"ecma262-compare — Compare the output HTML" line in the checks in arai-a/workflow-test#2
"Details" link leads to PR diff view.

But apparently posting PR status requires the user (@ecma262-compare-bot) to be a collaborator of the repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants