Thanks Weston!
> @Andrew What does "waiting for review" mean to you? Does "no existing
> reviewers and passes checks" sound reasonable?
Yes those and "not a draft PR"
> Is there a distinction between "never reviewed and ready" and "has been
reviewed but review is re-requested"?
Not for me -- once a PR has had a review, the author then has someone they
can @Mention <ment...@noreply.github.com> to move the process along if
needed.
Andrew
On Fri, Mar 12, 2021 at 10:34 PM Weston Pace <weston.p...@gmail.com> wrote:
> 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
> > > >
> > >
>
