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

Added a dashboard for tracking stale PRs #71

Closed ajbozarth closed 8 years ago

ajbozarth commented 8 years ago

Fixes #1

I added a new Stale PRs dash and sorting options for all, hanging, or abandoned PRs. These new dashes are still sortable by component as have all the features of the original dash.

Known behavioral oddity: When a component tab (such as Mesos) is selected and you you switch to a status tab (like Hanging or Abandoned) where there are no PRs in that component it will show the All components tab, but if you switch back to a status tab where the component exists it will switch back to it (instead of staying on all). This is caused by the default behavior of showing the All tab when the hash in the url matches an nonexistent tab.

Open Question: Is the UI ok? It works and doesn't look too bad, but the two sorting tab rows with the same color right on top of each other feels a bit off. What are some suggestions to fix this?

Screenshots: all-all all-build hang-all hang-build aband-all aband-build

nchammas commented 8 years ago

😮 Wow, great work!

I'm not a good person to review the code, but the UI looks fine to me. It's usable and practical, and if it encourages committers to review 5-10 PRs each week that they wouldn't have reviewed otherwise, that's a huge win.

ajbozarth commented 8 years ago

Oh also just remembered there's another possible issue to discuss. My method of determining hanging v abandoned uses the in-place commenters list. This means bots are ignored completely, so a PR last touched by a bot can be listed as either hanging or abandoned based on whoever commented previous to the bot.

nchammas commented 8 years ago

I think that's fine for starters. It's a great start, actually.

Later on we can probably improve this by tracking a list of committers somewhere so the hanging view, for example, will not count comments by non-committers. (e.g. If I open a PR and various people comment but no committer comments, that should probably count as a hanging PR that's waiting for committer input.)

ajbozarth commented 8 years ago

@JoshRosen @srowen @rxin as the committers who've been most invested in this issue, how do you feel about this?

srowen commented 8 years ago

Yeah, certainly seems worth trying out. I think anything to support more effective triage is worthwhile.

ajbozarth commented 8 years ago

@JoshRosen Is this something we want to merge or does it need additional work? The settings.cfg file will need to be updated once this is merged to match the template also.

ajbozarth commented 8 years ago

@JoshRosen Have you had a chance to take a look at this? Or an idea of when you will? Thanks

ajbozarth commented 8 years ago

@JoshRosen Now that you're back from vacation, any chance you'll look at this before Summit next week? No rush, just trying to gauge the status of my PRs.

ajbozarth commented 8 years ago

@JoshRosen now that Spark Summit is over when do you think you could take a look at this?

JoshRosen commented 8 years ago

Just had a chance to test this out and it looks fine to me, so I've pushed it to the live site and will merge this now. Thanks @ajbozarth!

ajbozarth commented 8 years ago

Thanks @JoshRosen