Compile checklist of PR review steps and more clearly share responsibility for readying PRs

[jywarren]

Looking to help check off pull request review steps and get the workflow more clear so we don't get backlogged and more people can help!

@publiclab/reviewers what are our steps?

• add label has-pull-request to the issue having a pr
• link to original issue
• description in title
• reviewed/approved by at least one reviewer
• screenshot if applicable
• passes tests

What else?

• label as ready

[SidharthBansal]

Please check modularity also.

[jywarren]

Yes, so:

• PR is (ideally) less than 50 lines of code & generally tries to implement only a single feature or fix
• PR doesn't affect highly sensitive areas of the site such as header or top of dashboard or signup form without input from @publiclab/community-reps
• Gemfile.lock, yarn.lock, and other unrelated files are not included (message below)

Hi, this is looking great, but would you mind removing the Gemfile.lock from your PR, as I think it's just a version tweak? https://stackoverflow.com/questions/215718/reset-or-revert-a-specific-file-to-a-specific-revision-using-git

How about these couple extra as well? Thanks!!!

[tech4GT]

@jywarren Awesome!! Lot's of nice things in this list!! How about these

• Issue should be claimed before opening the pull request
• Double check if it contains any feature that will be difficult to roll back(Possibly ask for review from other reviewers as well)

[grvsachdeva]

We also have this step in summer:

• label as review-me if review required from another reviewer

[jywarren]

We could start requiring 2 reviews, potentially -- and/or we could mark some people as reviewers who have deeper experience with the site and getting at least one review from this group... what are your thoughts? Thanks, all!

…

On Tue, Oct 2, 2018 at 3:01 PM Gaurav Sachdeva ***@***.***> wrote: We also have this step in summer: - label as review-me if review required from another reviewer — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3563 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJw2sgbia1cIPdEZmm7L6M7gU6A9Bks5ug7fmgaJpZM4XECaC> .

[tech4GT]

@jywarren @SidharthBansal @gauravano I think we should have a project specific team, since not everyone on plots is familiar with Image sequencer and vice versa, apart from the standard reviewers team let's have specific project specific teams with deeper experience with that project, like plots-experts and IS-experts
What say?
Thanks!

[SidharthBansal]

Instead of tagging we can direct Image sequencer to your review only. Leaflet to sagar's review only. Because no one else I assume have knowledge of those projects. After your review jeff can re- review them and merge them.
What you say?

[tech4GT]

@SidharthBansal I think making an experts team still makes sense since in future we may have others who could also get added to that, also there is @ccpandhare who knows the project inside out, and since @jywarren is the only one merging prs anyway, he can review them then. Thanks!

[SidharthBansal]

Actually I have not thought about future. I am really sorry I forget to think about future. Yeah your idea is better. I completely agree with you. Thanks

…

On Wed, Oct 3, 2018, 6:36 PM Varun Gupta ***@***.***> wrote: @SidharthBansal <https://github.com/SidharthBansal> I think making an experts team still makes sense since in future we may have others who could also get added to that, also there is @ccpandhare <https://github.com/ccpandhare> who knows the project inside out, and since @jywarren <https://github.com/jywarren> is the only one merging prs anyway, he can review them then. Thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3563 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AUACQ2yC7wMU_d_771P-emXfXv0A1jGEks5uhLZDgaJpZM4XECaC> .

[jywarren]

❤️ this convo. Could we think of a name a little less exclusive sounding than experts? I guess that is what the name reviewers is for, in part. Something else?

[jywarren]

Does everyone know how to use saved replies? It's really helpful to keep a consistent nice tone and provide common links. Sidharth maybe "install saved replies" is itself a task we could make?

[SidharthBansal]

We should just inform this to mentors, there is no need of creating an issue. This depends on individuals, not on community. If we may create an issue then there are potential chances that students will get confused.

…

On Thu, Oct 4, 2018, 1:22 AM Jeffrey Warren ***@***.***> wrote: Does everyone know how to use saved replies? It's really helpful to keep a consistent nice tone and provide common links. Sidharth maybe "install saved replies" is itself a task we could make? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3563 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AUACQ1SnZqz-SlrSOEZwozTZIsyZTY-9ks5uhRWOgaJpZM4XECaC> .