Re: Growing number of open PRs and Labelling PRs
A little more context and clarification: This is an effort to reduce the
number of open PRs and I along with others stated above as a first step
labeled the PRs to help us and the community to chime in and review. We(the
above) will be spending some time on reviewing some of the open PRs and
also invite the contributors/committers to participate.
Going forward we are looking into using the mxnet-label-bot to help
labeling the PRs as well.
On Thu, Aug 9, 2018 at 9:15 AM, sandeep krishnamurthy <
sandeep.krishn...@gmail.com> wrote:
> Hello Community,
>
> Thanks to our committers - Anirudh, Naveen and Haibin, we have now labelled
> all PRs raised in Jul/Aug (~80 PRs).
> PRs are labelled with current state (pr-awaiting-review, pr-awaiting-merge
> etc.) and components/functionality (Gluon, MKLDNN, Bugfix, Backend etc.)
> addressed in the PR. It should now be easy for you to filter PRs of your
> interest, expertise and awaiting-review.
>
> For example, we have around 50 PRs awaiting reviews -
> https://github.com/apache/incubator-mxnet/pulls?utf8=%
> E2%9C%93=is%3Apr+is%3Aopen+label%3Apr-awaiting-review
>
> Looking forward for all your contributions.
>
> Best,
> Sandeep
>
> On Wed, Aug 8, 2018 at 3:02 PM sandeep krishnamurthy <
> sandeep.krishn...@gmail.com> wrote:
>
> > @Sheng - Thanks for the feedback. I agree PR template provides that info.
> > But, one major draw back is ability to filter, group and take actions
> > (review, merge, ping corresponding contributors). And when we have
> deployed
> > the labelling bot (hopefully soon to happen), we should be able to
> > disassociate from the need for committer.
> > Suggestion?
> >
> > @Naveen - Thanks. I will make the change.
> >
> > On Wed, Aug 8, 2018 at 2:55 PM Naveen Swamy wrote:
> >
> >> suggest to change pr-ready-to-merge to pr-awaiting-merge? makes it easy
> to
> >> pick all pr related status.
> >>
> >> Also think `(then merge)` is not necessary in pr-awaiting-testing
> >>
> >> On Wed, Aug 8, 2018 at 2:44 PM, sandeep krishnamurthy <
> >> sandeep.krishn...@gmail.com> wrote:
> >>
> >> > << Sorry sent too early>>
> >> > Hello Community,
> >> >
> >> > Recently, we are observing a growing number of PR open {pending for
> >> review,
> >> > pending for updates, ready to merge but waiting and more}.
> >> >
> >> > Few of us committers (Naveen, Haibin, Anirudh and Me) and contributors
> >> > (Steffen and Hagay) met to discuss on how to improve the process in
> >> > reviewing the PR and allow more people join the review process.
> >> >
> >> > To shed some light on numbers:
> >> >
> >> > *(As of 6-Aug-2018)*
> >> >
> >> >- Total open PRs - 113 - Link
> >> >
> >> >- Total open PRs with No Reviews - 94 - Link
> >> > >> > 3Apr+is%3Aopen+review%3Anone>
> >> >(*Note:* Out of these there are comments for 72 PRs. This count is
> >> for
> >> >formally reviewing and approve/request change etc.)
> >> >
> >> >
> >> >- Changes Requested and awaiting contributors to update - 8 - Link
> >> > >> > 3Apr+is%3Aopen+review%3Achanges-requested>
> >> >- Oldest PR - Jan 19, 2018 - PR
> >> >
> >> >
> >> > One important issue observed is, "*Inability to filter the PR based on
> >> > state and component*". For this, one suggested solution is to "*label
> >> the
> >> > PRs*" like we label the issues. This will allow community members to
> >> filter
> >> > by area of interest, add review, committers can filter by state and
> take
> >> > necessary action.
> >> >
> >> > In this direction, I have created following 4 new labels.
> >> >
> >> > Please let us know your suggestions, and this is open for feedback and
> >> > changes.
> >> >
> >> >
> >> > -
> >> > pr-awaiting-review
> >> >
> >> > PR is waiting for code review
> >> > Edit Delete
> >> > - pr-awaiting-response
> >> > awaiting-response>
> >> > PR is reviewed and waiting for contributor to respond
> >> > Edit Delete
> >> > - pr-awaiting-testing (then merge)
> >> > >> > awaiting-testing%20%28then%20merge%29>
> >> > PR is reviewed and waiting CI build and test
> >> > Edit Delete
> >> > - pr-ready-to-merge
> >> >
> >> > Review and CI is complete. Ready to Merge
> >> >
> >> >
> >> > On Wed, Aug 8, 2018 at 2:35 PM sandeep krishnamurthy <
> >> > sandeep.krishn...@gmail.com> wrote:
> >> >
> >> > > Hello Community,
> >> > >
> >> > > Recently, we are observing a growing number of PR open {pending for
> >> > > review, pending for updates, ready to merge but waiting and more}.
> >> > >
> >> > > To shed some light on numbers:
> >> > >
> >> > > --
> >>
Re: Growing number of open PRs and Labelling PRs
Hello Community,
Thanks to our committers - Anirudh, Naveen and Haibin, we have now labelled
all PRs raised in Jul/Aug (~80 PRs).
PRs are labelled with current state (pr-awaiting-review, pr-awaiting-merge
etc.) and components/functionality (Gluon, MKLDNN, Bugfix, Backend etc.)
addressed in the PR. It should now be easy for you to filter PRs of your
interest, expertise and awaiting-review.
For example, we have around 50 PRs awaiting reviews -
https://github.com/apache/incubator-mxnet/pulls?utf8=%E2%9C%93=is%3Apr+is%3Aopen+label%3Apr-awaiting-review
Looking forward for all your contributions.
Best,
Sandeep
On Wed, Aug 8, 2018 at 3:02 PM sandeep krishnamurthy <
sandeep.krishn...@gmail.com> wrote:
> @Sheng - Thanks for the feedback. I agree PR template provides that info.
> But, one major draw back is ability to filter, group and take actions
> (review, merge, ping corresponding contributors). And when we have deployed
> the labelling bot (hopefully soon to happen), we should be able to
> disassociate from the need for committer.
> Suggestion?
>
> @Naveen - Thanks. I will make the change.
>
> On Wed, Aug 8, 2018 at 2:55 PM Naveen Swamy wrote:
>
>> suggest to change pr-ready-to-merge to pr-awaiting-merge? makes it easy to
>> pick all pr related status.
>>
>> Also think `(then merge)` is not necessary in pr-awaiting-testing
>>
>> On Wed, Aug 8, 2018 at 2:44 PM, sandeep krishnamurthy <
>> sandeep.krishn...@gmail.com> wrote:
>>
>> > << Sorry sent too early>>
>> > Hello Community,
>> >
>> > Recently, we are observing a growing number of PR open {pending for
>> review,
>> > pending for updates, ready to merge but waiting and more}.
>> >
>> > Few of us committers (Naveen, Haibin, Anirudh and Me) and contributors
>> > (Steffen and Hagay) met to discuss on how to improve the process in
>> > reviewing the PR and allow more people join the review process.
>> >
>> > To shed some light on numbers:
>> >
>> > *(As of 6-Aug-2018)*
>> >
>> >- Total open PRs - 113 - Link
>> >
>> >- Total open PRs with No Reviews - 94 - Link
>> >> > 3Apr+is%3Aopen+review%3Anone>
>> >(*Note:* Out of these there are comments for 72 PRs. This count is
>> for
>> >formally reviewing and approve/request change etc.)
>> >
>> >
>> >- Changes Requested and awaiting contributors to update - 8 - Link
>> >> > 3Apr+is%3Aopen+review%3Achanges-requested>
>> >- Oldest PR - Jan 19, 2018 - PR
>> >
>> >
>> > One important issue observed is, "*Inability to filter the PR based on
>> > state and component*". For this, one suggested solution is to "*label
>> the
>> > PRs*" like we label the issues. This will allow community members to
>> filter
>> > by area of interest, add review, committers can filter by state and take
>> > necessary action.
>> >
>> > In this direction, I have created following 4 new labels.
>> >
>> > Please let us know your suggestions, and this is open for feedback and
>> > changes.
>> >
>> >
>> > -
>> > pr-awaiting-review
>> >
>> > PR is waiting for code review
>> > Edit Delete
>> > - pr-awaiting-response
>> >
>> > PR is reviewed and waiting for contributor to respond
>> > Edit Delete
>> > - pr-awaiting-testing (then merge)
>> > > > awaiting-testing%20%28then%20merge%29>
>> > PR is reviewed and waiting CI build and test
>> > Edit Delete
>> > - pr-ready-to-merge
>> >
>> > Review and CI is complete. Ready to Merge
>> >
>> >
>> > On Wed, Aug 8, 2018 at 2:35 PM sandeep krishnamurthy <
>> > sandeep.krishn...@gmail.com> wrote:
>> >
>> > > Hello Community,
>> > >
>> > > Recently, we are observing a growing number of PR open {pending for
>> > > review, pending for updates, ready to merge but waiting and more}.
>> > >
>> > > To shed some light on numbers:
>> > >
>> > > --
>> > > Sandeep Krishnamurthy
>> > >
>> >
>> >
>> > --
>> > Sandeep Krishnamurthy
>> >
>>
>
>
> --
> Sandeep Krishnamurthy
>
--
Sandeep Krishnamurthy
Re: Growing number of open PRs and Labelling PRs
@Sheng - Thanks for the feedback. I agree PR template provides that info.
But, one major draw back is ability to filter, group and take actions
(review, merge, ping corresponding contributors). And when we have deployed
the labelling bot (hopefully soon to happen), we should be able to
disassociate from the need for committer.
Suggestion?
@Naveen - Thanks. I will make the change.
On Wed, Aug 8, 2018 at 2:55 PM Naveen Swamy wrote:
> suggest to change pr-ready-to-merge to pr-awaiting-merge? makes it easy to
> pick all pr related status.
>
> Also think `(then merge)` is not necessary in pr-awaiting-testing
>
> On Wed, Aug 8, 2018 at 2:44 PM, sandeep krishnamurthy <
> sandeep.krishn...@gmail.com> wrote:
>
> > << Sorry sent too early>>
> > Hello Community,
> >
> > Recently, we are observing a growing number of PR open {pending for
> review,
> > pending for updates, ready to merge but waiting and more}.
> >
> > Few of us committers (Naveen, Haibin, Anirudh and Me) and contributors
> > (Steffen and Hagay) met to discuss on how to improve the process in
> > reviewing the PR and allow more people join the review process.
> >
> > To shed some light on numbers:
> >
> > *(As of 6-Aug-2018)*
> >
> >- Total open PRs - 113 - Link
> >
> >- Total open PRs with No Reviews - 94 - Link
> > > 3Apr+is%3Aopen+review%3Anone>
> >(*Note:* Out of these there are comments for 72 PRs. This count is for
> >formally reviewing and approve/request change etc.)
> >
> >
> >- Changes Requested and awaiting contributors to update - 8 - Link
> > > 3Apr+is%3Aopen+review%3Achanges-requested>
> >- Oldest PR - Jan 19, 2018 - PR
> >
> >
> > One important issue observed is, "*Inability to filter the PR based on
> > state and component*". For this, one suggested solution is to "*label the
> > PRs*" like we label the issues. This will allow community members to
> filter
> > by area of interest, add review, committers can filter by state and take
> > necessary action.
> >
> > In this direction, I have created following 4 new labels.
> >
> > Please let us know your suggestions, and this is open for feedback and
> > changes.
> >
> >
> > -
> > pr-awaiting-review
> >
> > PR is waiting for code review
> > Edit Delete
> > - pr-awaiting-response
> >
> > PR is reviewed and waiting for contributor to respond
> > Edit Delete
> > - pr-awaiting-testing (then merge)
> > > awaiting-testing%20%28then%20merge%29>
> > PR is reviewed and waiting CI build and test
> > Edit Delete
> > - pr-ready-to-merge
> >
> > Review and CI is complete. Ready to Merge
> >
> >
> > On Wed, Aug 8, 2018 at 2:35 PM sandeep krishnamurthy <
> > sandeep.krishn...@gmail.com> wrote:
> >
> > > Hello Community,
> > >
> > > Recently, we are observing a growing number of PR open {pending for
> > > review, pending for updates, ready to merge but waiting and more}.
> > >
> > > To shed some light on numbers:
> > >
> > > --
> > > Sandeep Krishnamurthy
> > >
> >
> >
> > --
> > Sandeep Krishnamurthy
> >
>
--
Sandeep Krishnamurthy
Re: Growing number of open PRs and Labelling PRs
suggest to change pr-ready-to-merge to pr-awaiting-merge? makes it easy to
pick all pr related status.
Also think `(then merge)` is not necessary in pr-awaiting-testing
On Wed, Aug 8, 2018 at 2:44 PM, sandeep krishnamurthy <
sandeep.krishn...@gmail.com> wrote:
> << Sorry sent too early>>
> Hello Community,
>
> Recently, we are observing a growing number of PR open {pending for review,
> pending for updates, ready to merge but waiting and more}.
>
> Few of us committers (Naveen, Haibin, Anirudh and Me) and contributors
> (Steffen and Hagay) met to discuss on how to improve the process in
> reviewing the PR and allow more people join the review process.
>
> To shed some light on numbers:
>
> *(As of 6-Aug-2018)*
>
>- Total open PRs - 113 - Link
>
>- Total open PRs with No Reviews - 94 - Link
> 3Apr+is%3Aopen+review%3Anone>
>(*Note:* Out of these there are comments for 72 PRs. This count is for
>formally reviewing and approve/request change etc.)
>
>
>- Changes Requested and awaiting contributors to update - 8 - Link
> 3Apr+is%3Aopen+review%3Achanges-requested>
>- Oldest PR - Jan 19, 2018 - PR
>
>
> One important issue observed is, "*Inability to filter the PR based on
> state and component*". For this, one suggested solution is to "*label the
> PRs*" like we label the issues. This will allow community members to filter
> by area of interest, add review, committers can filter by state and take
> necessary action.
>
> In this direction, I have created following 4 new labels.
>
> Please let us know your suggestions, and this is open for feedback and
> changes.
>
>
> -
> pr-awaiting-review
>
> PR is waiting for code review
> Edit Delete
> - pr-awaiting-response
>
> PR is reviewed and waiting for contributor to respond
> Edit Delete
> - pr-awaiting-testing (then merge)
> awaiting-testing%20%28then%20merge%29>
> PR is reviewed and waiting CI build and test
> Edit Delete
> - pr-ready-to-merge
>
> Review and CI is complete. Ready to Merge
>
>
> On Wed, Aug 8, 2018 at 2:35 PM sandeep krishnamurthy <
> sandeep.krishn...@gmail.com> wrote:
>
> > Hello Community,
> >
> > Recently, we are observing a growing number of PR open {pending for
> > review, pending for updates, ready to merge but waiting and more}.
> >
> > To shed some light on numbers:
> >
> > --
> > Sandeep Krishnamurthy
> >
>
>
> --
> Sandeep Krishnamurthy
>
Re: Growing number of open PRs and Labelling PRs
Hi Sandeep,
Sorry if I asked an obvious question, but is it required to introduce a new
solution that require committer access? We have an existing solution to
communicate the PR status, which is through the PR template checklist. The
PR checklist provides clickable options to reflect the PR status. It
doesn't require committer involvement and contributors can directly edit
the description themselves. It shows a progress bar that can be seen
directly in the pull requests page.
https://github.com/apache/incubator-mxnet/pulls
Best regards,
-sz
On Wed, Aug 8, 2018 at 2:44 PM, sandeep krishnamurthy <
sandeep.krishn...@gmail.com> wrote:
> << Sorry sent too early>>
> Hello Community,
>
> Recently, we are observing a growing number of PR open {pending for review,
> pending for updates, ready to merge but waiting and more}.
>
> Few of us committers (Naveen, Haibin, Anirudh and Me) and contributors
> (Steffen and Hagay) met to discuss on how to improve the process in
> reviewing the PR and allow more people join the review process.
>
> To shed some light on numbers:
>
> *(As of 6-Aug-2018)*
>
>- Total open PRs - 113 - Link
>
>- Total open PRs with No Reviews - 94 - Link
> 3Apr+is%3Aopen+review%3Anone>
>(*Note:* Out of these there are comments for 72 PRs. This count is for
>formally reviewing and approve/request change etc.)
>
>
>- Changes Requested and awaiting contributors to update - 8 - Link
> 3Apr+is%3Aopen+review%3Achanges-requested>
>- Oldest PR - Jan 19, 2018 - PR
>
>
> One important issue observed is, "*Inability to filter the PR based on
> state and component*". For this, one suggested solution is to "*label the
> PRs*" like we label the issues. This will allow community members to filter
> by area of interest, add review, committers can filter by state and take
> necessary action.
>
> In this direction, I have created following 4 new labels.
>
> Please let us know your suggestions, and this is open for feedback and
> changes.
>
>
> -
> pr-awaiting-review
>
> PR is waiting for code review
> Edit Delete
> - pr-awaiting-response
>
> PR is reviewed and waiting for contributor to respond
> Edit Delete
> - pr-awaiting-testing (then merge)
> awaiting-testing%20%28then%20merge%29>
> PR is reviewed and waiting CI build and test
> Edit Delete
> - pr-ready-to-merge
>
> Review and CI is complete. Ready to Merge
>
>
> On Wed, Aug 8, 2018 at 2:35 PM sandeep krishnamurthy <
> sandeep.krishn...@gmail.com> wrote:
>
> > Hello Community,
> >
> > Recently, we are observing a growing number of PR open {pending for
> > review, pending for updates, ready to merge but waiting and more}.
> >
> > To shed some light on numbers:
> >
> > --
> > Sandeep Krishnamurthy
> >
>
>
> --
> Sandeep Krishnamurthy
>
Re: Growing number of open PRs and Labelling PRs
<< Sorry sent too early>>
Hello Community,
Recently, we are observing a growing number of PR open {pending for review,
pending for updates, ready to merge but waiting and more}.
Few of us committers (Naveen, Haibin, Anirudh and Me) and contributors
(Steffen and Hagay) met to discuss on how to improve the process in
reviewing the PR and allow more people join the review process.
To shed some light on numbers:
*(As of 6-Aug-2018)*
- Total open PRs - 113 - Link
- Total open PRs with No Reviews - 94 - Link
(*Note:* Out of these there are comments for 72 PRs. This count is for
formally reviewing and approve/request change etc.)
- Changes Requested and awaiting contributors to update - 8 - Link
- Oldest PR - Jan 19, 2018 - PR
One important issue observed is, "*Inability to filter the PR based on
state and component*". For this, one suggested solution is to "*label the
PRs*" like we label the issues. This will allow community members to filter
by area of interest, add review, committers can filter by state and take
necessary action.
In this direction, I have created following 4 new labels.
Please let us know your suggestions, and this is open for feedback and
changes.
-
pr-awaiting-review
PR is waiting for code review
Edit Delete
- pr-awaiting-response
PR is reviewed and waiting for contributor to respond
Edit Delete
- pr-awaiting-testing (then merge)
PR is reviewed and waiting CI build and test
Edit Delete
- pr-ready-to-merge
Review and CI is complete. Ready to Merge
On Wed, Aug 8, 2018 at 2:35 PM sandeep krishnamurthy <
sandeep.krishn...@gmail.com> wrote:
> Hello Community,
>
> Recently, we are observing a growing number of PR open {pending for
> review, pending for updates, ready to merge but waiting and more}.
>
> To shed some light on numbers:
>
> --
> Sandeep Krishnamurthy
>
--
Sandeep Krishnamurthy
