search

databricks / spark-pr-dashboard

Dashboard to aid in Spark pull request reviews
spark-prs.appspot.com
Apache License 2.0
54 stars 46 forks source link

Add "committer commented" column #78

Closed JoshRosen closed 7 years ago

JoshRosen commented 7 years ago

This patch adds an "Committer Commented" column to the PRTableView which shows the timestamp of the most recent comment from any Spark committer.

JoshRosen commented 7 years ago

This change is currently deployed in case you want to test it out.

ajbozarth commented 7 years ago

I like the addition but it's awkward visually with each word in "Updated by Committer" wrapping to its own line. Maybe change it to "Committer Updated" or "Committer Commented" so it only wraps to two line like other headers?

Also The Stale PR tab isn't filling in the column, I think refreshStalePrs needs to be updated to match changes to refreshPrs in AppManager.js

jkbradley commented 7 years ago

I like "Committer Commented" since it's really about comments. (The PR author could be a committer, but code updates are not counted in this column.)

I'm not one to review Javascript, but apart from the bug @ajbozarth pointed out, this LGTM when I use it on the site. Thank you!

ajbozarth commented 7 years ago

Outside my previous comment the code LGTM as well

JoshRosen commented 7 years ago

Thanks for reviewing. I've updated this to incorporate your feedback.

ajbozarth commented 7 years ago

LGTM

ajbozarth commented 7 years ago

So I just noticed an odd behavior that this update made more obvious, you can currently see it on the main page. If there is both a pr with a long word in its title and another with a long user name and it's making the table wider than the page width causing a horizontal scroll.

JoshRosen commented 7 years ago

Ah, that's unfortunate. It would be neat if we could truncate long titles in the frontend using Javascript with the overflow: ellipsis CSS option. Feel free to give it a try and submit a PR and I'll review.