So I took some time to look this over today and consider the
possibilities. I also explored the basic capabilities available from
Github more and learned a few tricks and capabilities that I didn't
realize existed.
@Wes I'm not sure how the Spark dashboard is helpful. This may be a
case of simply not understanding the intended workflow but it seems
like a more cluttered version of Github's own PR dashboard. The
columns that are unique to the dashboard (like shepherd or commenters)
are either unknown to me (shepherd) or I don't understand how it is
helpful (commenters). You can filter by component but Github's
dashboard allows filtering by label as well and we have labels for all
the components. So at the moment I am leaning towards enriching
Github vs creating something new.
@Andrew What does "waiting for review" mean to you? Does "no existing
reviewers and passes checks" sound reasonable? Is there a distinction
between "never reviewed and ready" and "has been reviewed but review
is re-requested"? Github already has a "no reviews" filter so I think
if there were a "ready-for-review" label (such a label actually exists
in the repo but is not used) then that should be doable.
@Neal Andrew is correct, these labels would need to be applied by
Github Actions. However, the actions API is pretty extensive and
capable of all the labels I'm suggesting.
There are a few potential labels I am considering. Does anyone have
any strong feelings on...
* Size labels - There are existing Github actions to label PRs based
on their size (e.g. making a "tiny" label if the PR is less than 100
lines changed). The theory being (presumably) that you can quickly
address the tiny reviews and get them out of your way
* Ready for review - I mentioned this above but this label would be
applied if checks are passing and either there are no reviews or all
reviews have been re-requested
* Needs attention - If a PR is in a "ready for review" state for some
number of days then it could be flagged as "needs attention" to help
make sure someone acts on it
* First time contributor - If a PR is from a user that has never
contributed to the repository before this label could be added,
similar to Stack Overflows "first time user" label
In addition to labels, we could add the ability for users to perform
certain actions using comments (github actions can watch comments).
Suggested:
* Needs review - By typing "please review" the user can have the
"ready for review" label applied (the help comment that is applied to
new issues could include these directions). This could be useful if
we decide to not automatically apply "ready for review" or if a user
decides a failing check is invalid / unrelated.
* Ignore failing checks - By typing "please ignore failing checks" the
user can have the "ready for review" label applied even if there are
failing checks. This is somewhat mutually exclusive with the above
item but probably necessary if we decide to automatically apply the
ready for review label.
On Mon, Mar 8, 2021 at 12:08 PM Andrew Lamb <al...@influxdata.com> wrote:
>
> @Weston: Adding a system to figure out when PRs are waiting for review
> would really help me personally. I spend non trivial time trying to make
> sure we don't miss any Rust PRs that come in and it gets challenging to
> filter out the ones that are WIP.
>
> Having a bot add some labels would be great.
>
> @Neil: Prior to being a committer I could not change labels on PRs, so I
> don't think it is possible. Perhaps the bot could add the labels on their
> behalf based on comments ("bot this is ready for review" or something)
>
>
> On Sun, Mar 7, 2021 at 10:28 PM Neal Richardson <neal.p.richard...@gmail.com>
> wrote:
>
> > Are non-committers able to add/remove tags to PRs? Sounds like a good way
> > to increase visibility/searchability of PRs needing review, but it would
> > only work if everyone is authorized to modify tags.
> >
> > Neal
> >
> > On Sun, Mar 7, 2021 at 12:24 PM Wes McKinney <wesmck...@gmail.com> wrote:
> >
> > > This sounds like a nice idea as long as the bot doesn't generate too
> > > much potentially spurious info (i.e. the "needs improvement" label
> > > when it perhaps need not).
> > >
> > > Note that the Spark community set up a pretty handy PR dashboard
> > >
> > > https://spark-prs.appspot.com
> > >
> > > It's open source: https://github.com/databricks/spark-pr-dashboard
> > >
> > > Frankly I think we are getting to (and probably well past) the point
> > > that scrolling through > 100 PRs in GitHub is not a good use of
> > > reviewer time, so creating some kind of "semantic layer" on top of the
> > > PR review queue (like what the Spark folks did) would help a great
> > > deal. So rather than trying to work entirely within the GitHub
> > > platform (which is not designed for the needs of large projects with
> > > this many PRs), something more structured and useful could be created.
> > > If you look at the Spark dashboard, that is 10x more helpful than
> > > browsing GitHub is right now, whether there are issue labels or not.
> > >
> > > On Sat, Mar 6, 2021 at 10:41 PM Weston Pace <weston.p...@gmail.com>
> > wrote:
> > > >
> > > > Since we're on the topic of improving the workflow I had a proposal to
> > > > consider. Right now the review process is pretty ad-hoc which is fine
> > > > but can be a bit tricky to keep track of.
> > > >
> > > > I think it could be improved with a bot to label PRs. The labels would
> > > be...
> > > >
> > > > state: failing tests - Any PR starts in this state until either all
> > > > tests pass or the submitter could say something like "please review"
> > > > to bypass (e.g. in case an intermittent or unrelated test failed since
> > > > non-committers are not able to rerun jobs). Add something to
> > > > developer docs FAQ about this bypass.
> > > >
> > > > state: needs review - Once all tests pass a PR would move into this
> > > state.
> > > >
> > > > state: needs improvement - If someone makes a review with the
> > > > "comment" or "request changes" action the PR moves into this state.
> > > > To clear this state the user must click "re-request review". Add
> > > > something to developer docs FAQ about this since it may be
> > > > unintuitive.
> > > >
> > > > That's it. I'd be happy to create the bot if the community thinks it
> > > > is a good idea. None of this would be required or authoritative.
> > > > It's merely a label to help for tracking purposes. Reviewers could
> > > > presumably use the tags to create some kind of dashboard showing which
> > > > PRs need review and which have been waiting for review the longest.
> > > > Even without that, it would serve to make it clear to the submitter
> > > > and the reviewers where the action is.
> > > >
> > > > -Weston Pace
> > >
> >
