Thinking about rendering compat tables for Pull Requests

connorshea commented 6 years ago

I've been thinking about how to implement a simple system for previewing the browser compat tables directly from the PR, and wanted to get my thoughts out in an issue. I'm not sure if this kind of thing fits better in the browser-compat-toolkit project?

Goal

To render browser compat tables based on the JSON files changed in each Pull Request to this repository.

Options

There are a few ways this could be done:

Render the table in a GitHub comment with Markdown.
Render the table as a static site, take a screenshot using Chrome Driver, and post a comment with the image (also possibly attach the HTML/CSS/JS so anyone can run the static site locally to test it?)
Render the table as a static site hosted somewhere?

Outline

My thinking is that the second option is the best, so I wanted to think about a rough outline of how it might work:

Run DangerJS in Travis CI for every Pull Request.
- tl;dr Danger is a tool for automatically posting GitHub comments on Pull Requests from your CI environment. It has access to a bunch of the pull request's metadata so it can determine what JSON files a PR changes and render the table from that information (in theory, anyway).
- A better explanation of DangerJS is available on its website.
Have Danger parse the changed JSON files and render the table(s).
- The table could possibly be rendered using existing JavaScript in the KumaScript repository.
- I'm not sure how much of the table is rendered server-side by Django vs. client-side with JavaScript, will have to look into that.
Run Chrome Driver to get a screenshot of the page, pass it to Danger, and post it as part of the comment.

connorshea commented 6 years ago

I managed to get as far as generating this page, ugly but promising!

connorshea commented 6 years ago

Okay, I made it all the way to being able to comment on a PR using Danger, and now I find out you can't upload images to GitHub via their API. ಠ_ಠ

connorshea commented 6 years ago

I got it to work 👍

https://github.com/connorshea/browser-compat-data/pull/3#issuecomment-390446997

Now whenever a new PR comes in that changes a JSON file, CI will create a comment in my repository that includes a screenshot of the generated table. Right now it uploads to imgur, and the code is terrible since I was just trying to force it to work, but it works!

ExE-Boss commented 6 years ago

We could use browser-compat-toolkit for this, in fact, this is listed as one of its goals.

connorshea commented 6 years ago

@ExE-Boss Yup! But it hasn't made a lot of progress so I wanted to try throwing together an MVP for this :)

Elchi3 commented 6 years ago

We thought about this as well at our Paris Hackathon in March. Happy to see some new experiments for this :) Nice work, Connor!

@ExE-Boss is right, we intend to use browser-compat-toolkit for this. However, with limited staff resources in the MDN team, it is currently not a project that we can afford to work on. I hope to get it on the roadmap sometime soon, though. In the meantime, I'm really happy about any experiments which we can consider to integrate into project(s)/repo(s) in the future (proof of concepts help a lot!), but I'm asking for some patience. Additional feedback and reviews on PRs on these things from the MDN team isn't always doable with the current projects we're already committed to finish first. Thank you :)

connorshea commented 6 years ago

@Elchi3 sorry if I came off as impatient, I can completely relate to being limited by time and priorities 🙇 This issue's purpose is just to provide my thoughts as well as an MVP so others can build off it in the future :)

foolip commented 5 years ago

@connorshea after finding the npm run render command today I had the same thought, and @Elchi3 pointed me at this issue. Is this something you're still pursuing?

foolip commented 5 years ago

FWIW, in web-platform-tests, we've had bots commenting on PRs and in our case it caused a fair amount of complaints: https://github.com/web-platform-tests/wpt/issues/5200 https://github.com/web-platform-tests/wpt/issues/5611 https://github.com/web-platform-tests/wpt/issues/5916

Perhaps PRs on this repo don't have enough iterations for that to be a problem, but the solution we ended up with for WPT was a custom-built check that @lukebjerring built. https://github.com/web-platform-tests/wpt/pull/15350/checks?check_run_id=62152674 is an example of that.

That's quite a lot of work, of course. Hopefully in the future someone will provide custom checks as a service, where one only needs to provide the output (text and images) and not the hosting...

connorshea commented 5 years ago

@foolip I’m not really pursuing it currently, feel free to take over if you like :)

foolip commented 5 years ago

That's too bad, but thanks for making the status clear :)

@Elchi3 @jpmedley how do you go about reviewing changes, is it as time consuming as it would seem to figure out from diff context what's changing and if it makes sense together with the other data in the file?

I expect that https://github.com/mdn/browser-compat-data/issues/3308 could result in some PRs that only update part of the support for an interface, and there it would help to be able to see more of the context at a glance I think.

ExE-Boss commented 5 years ago

It might be possible to get this implemented using mdn‑browser‑compat‑toolkit, even if it’s currently not being actively maintained.

Elchi3 commented 5 years ago

Looking at rendered tables would certainly help reviewing and possibly speed it up quite well. See also this cumbersome workaround https://discourse.mozilla.org/t/how-to-view-bcd-changes-as-rendered-on-mdn/32489 for getting rendered tables.

I think a "compatrender" bot that you could ask to render a table for you on request would be nicer than to add a rendering to every PR. That bot could then also accept a query much like the macro call on the wiki pages {{compat("css.properties.background-color")}}.

mdn‑browser‑compat‑toolkit was aiming to do some of this work, yes. The idea was also to have a generic renderer that could be used for MDN, for a GitHub bot, or even for a webcomponent or so that you could embed in your blog post and it would always have fresh compat data and not the data you had when you wrote the blog post. It was a hackathon idea, but hasn't been resourced since. You can try to resurrect browser-compat-toolkit as a fork or start fresh somewhere with this idea.

Or you just focus on the bot and have a more minimal rendering (but maybe somewhat close to the MDN rendering) much like Connor tried earlier. That would already be helpful as well I reckon.

jpmedley commented 5 years ago

I affectively review everything on the MDN page by hand.

This is not a significant burden in any way.

The reason is that I am almost always updating BCD in the context of an MDN page update. I always wait for the new data to be deployed. Then I go to the pages where it's rendered to either write an update or verify an update that's already written. Glancing at the compat table while I'm there is not a noticeable burden on my workload.

mdn / browser-compat-data