-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Summarize PR changes for ease of review #6863
Description
When reviewing a PR with a lot of changes, I spend most of the time expanding the diff to see which entry is being modified. This is especially annoying when reviewing changes towards the end of the browser list, where multiple clicks are needed to reveal the relevant context.
@vinyldarkscratch and I have discussed a GitHub Action workflow to produce a summary of changes as a PR comment (or something) to speed things up. I have written a proof-of-concept npm run diff in #6862 which I've used on a few PRs today, and found that indeed it increased my review velocity by a lot.
Here's a sample of what it currently looks like for #6838:
api.Window.blur
webview_android support set to 1 (was true)
api.Window.cancelAnimationFrame
chrome support set to 24 (was true)
chrome_android support set to 25 (was true)
samsunginternet_android support set to 1.5 (was true)
webview_android support set to ≤37 (was true)
api.Window.crypto
chrome support set to 1 (was 37)
chrome_android support set to 18 (was 37)
opera support set to 15 (was 24)
opera_android support set to 14 (was 24)
samsunginternet_android support set to 1.0 (was 3.0)
webview_android support set to 1 (was 37)
api.Window.document
chrome support set to 1 (was true)
chrome_android support set to 18 (was true)
samsunginternet_android support set to 1.0 (was true)
webview_android support set to 1 (was true)
api.Window.external
chrome support set to 1 (was true)
chrome_android support set to 18 (was true)
samsunginternet_android support set to 1.0 (was true)
webview_android support set to 1 (was true)
api.Window.find
webview_android support set to 1 (was true)
api.Window.focus
webview_android support set to 1 (was true)
I could quickly spot when data for other browsers than expected was being modified, and most importantly I didn't have to spend as much time understanding the diff context.
If others think this might be useful, there are a number of questions before making it real:
- Where should the summary be shown? Options are modifying the PR description, posting a new comment, inline annotations on the individual files, or as the output of a check.
- What's a good way to visualize the changes? Options that come to mind are a table with entry/browser/before/after columns, or as a single compat table with the changes somehow highlighted within, perhaps as insertions and
deletions.
The choices will inform how this has to be implemented. Thoughts appreciated.