<div dir="ltr"><div>Respected Sir,</div><br><div>Thank you very much for giving this opprtunity. We are very happy to join as a reviewer<br></div><div>I have understood the guidelines and the links you provided. I will start looking at existing reviews, before making any, to ensure consistency <br></div><div><br></div>Thanking you once again<br><br>Rahul Bothra<br><div>@Pro-Panda</div><div><br></div><div>+91-7733052890<br></div><div><br></div><div><div>> <span class="gmail-im">Comments? Should the above be added to sugar-docs?</span><br></div><div>Although added already, its a yes in my opinion as well.</div></div><div><br></div><div><br></div><div class="gmail_extra"><div class="gmail_quote">On Fri, Feb 23, 2018 at 5:10 AM, James Cameron <span dir="ltr"><<a href="mailto:quozl@laptop.org" target="_blank">quozl@laptop.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Added as<br> <a href="https://github.com/sugarlabs/sugar-docs/blob/master/src/contributing.md#guide-for-reviewers" rel="noreferrer" target="_blank">https://github.com/sugarlabs/s<wbr>ugar-docs/blob/master/src/cont<wbr>ributing.md#guide-for-reviewer<wbr>s</a><br> <br> Also, if you see me review in a way you don't understand, please ask.<br> <br> One can _always_ find something to criticise in a review, but one<br> should also choose if it is worth the time to do so. ;-)<br> <br> My underlying habits also arise from face to face source code reviews<br> in an ISO9001 quality control system; in the system we used at Digital<br> Equipment Corporation a reviewer could only say what was wrong, and<br> was restrained from saying how to fix it unless asked. This maintains<br> the authority and agency of the coder. But can unnecessarily delay<br> merge.<br> <span><br> On Fri, Feb 23, 2018 at 04:16:08AM +0530, Yash Agrawal wrote:<br> > Thank you James for the invitation. We are more than happy to join the<br> > organisation and help communtiy in reviewing and merging pull requests. :)<br> ><br> > Thank you for providing such detailed information, I have been through all the<br> > links you have provided. I understand the role of a reviewer much better now.<br> ><br> > >Should the above be added to sugar-docs?<br> > Very well written guidelines for any new member to the organisation. +1 for<br> > sugar-docs from me.<br> ><br> > Now that my exams are over, I look forward to a more active participation.<br> > cheers!<br> ><br> </span><span>> On Fri, Feb 23, 2018 at 3:25 AM, James Cameron <[1]<a href="mailto:quozl@laptop.org" target="_blank">quozl@laptop.org</a>> wrote:<br> ><br> > Rahul and Yash, your code contributions have been consistently good<br> > for the past month, so I've invited you to the GitHub sugarlabs<br> > organisation so that you can review and merge pull requests.<br> ><br> > Information below may be of help to guide you in this task.<br> ><br> > My goals for review are;<br> ><br> > - detect trivial mistakes,<br> ><br> > - maintain consistent and good code quality,<br> ><br> > - reproduce test results, (especially for critical repositories),<br> ><br> > - maintain a useful git commit history for use by git bisect, and<br> > developers who read it,<br> ><br> > - maintain other records, such as issues, tickets, and documentation,<br> ><br> > - not waste the time of the contributor, by doing myself anything<br> > trivial that otherwise the contributor might have to do.<br> ><br> > Checklist for review of pull requests;<br> ><br> > - [ ] does the change have consensus of the community,<br> </span>> [2]<a href="https://github.com/sugarlabs/sugar-docs/blob/master/src/CODE" rel="noreferrer" target="_blank">https://github.com/sugarlab<wbr>s/sugar-docs/blob/master/src/<wbr>CODE</a><br> <span>> _OF_CONDUCT.md<br> > (if a reviewer is in doubt, seek opinions by @mentioning people)<br> ><br> > - [ ] does the commit message explain the summary, problem, and<br> > solution, so that it can be used in future analysis,<br> </span>> [3]<a href="https://github.com/sugarlabs/sugar-docs/blob/master/src/cont" rel="noreferrer" target="_blank">https://github.com/sugarlab<wbr>s/sugar-docs/blob/master/src/<wbr>cont</a><br> <span>> <a href="http://ributing.md#making-commits" rel="noreferrer" target="_blank">ributing.md#making-commits</a><br> > (if a reviewer can fix it by squash or manual rebase, do so)<br> ><br> </span>> - [ ] does the commit message reference the issue, [4]<a href="http://bugs.sugarlabs.org" rel="noreferrer" target="_blank">bugs.sugarlabs.org</a><br> <span>> ticket number, or downstream ticket numbers,<br> > (if a reviewer can fix it by squash or manual rebase, do so)<br> ><br> > - [ ] are the number of commits excessive for future analysis,<br> > (a reviewer may squash or rebase if necessary)<br> ><br> > - [ ] is the changed code consistent in style with the existing code,<br> </span>> [5]<a href="https://github.com/sugarlabs/sugar-docs/blob/master/src/desk" rel="noreferrer" target="_blank">https://github.com/sugarlab<wbr>s/sugar-docs/blob/master/src/<wbr>desk</a><br> <span>> <a href="http://top-activity.md#coding-standards" rel="noreferrer" target="_blank">top-activity.md#coding-standa<wbr>rds</a><br> > (on the other hand, expect flake8 changes to be in separate commits)<br> ><br> > - [ ] for critical repositories, does the change work properly on our<br> > latest version of Sugar on either Fedora, Debian, or Ubuntu.<br> ><br> > Critical repositories are;<br> ><br> > - sugar, sugar-toolkit, sugar-toolkit-gtk3, sugar-artwork,<br> > sugar-datastore, gst-plugins-espeak,<br> ><br> > - each of the Fructose activity set repositories,<br> </span>> [6]<a href="https://wiki.sugarlabs.org/go/Development_Team/Release/Modules#" rel="noreferrer" target="_blank">https://wiki.sugarlabs.org/<wbr>go/Development_Team/Release/Mo<wbr>dules#</a><br> <span>> Fructose<br> ><br> > Comments? Should the above be added to sugar-docs?<br> ><br> > --<br> > James Cameron<br> </span>> [7]<a href="http://quozl.netrek.org/" rel="noreferrer" target="_blank">http://quozl.netrek.org/</a><br> ><br> > References:<br> ><br> > [1] mailto:<a href="mailto:quozl@laptop.org" target="_blank">quozl@laptop.org</a><br> > [2] <a href="https://github.com/sugarlabs/sugar-docs/blob/master/src/CODE_OF_CONDUCT.md" rel="noreferrer" target="_blank">https://github.com/sugarlabs/s<wbr>ugar-docs/blob/master/src/CODE<wbr>_OF_CONDUCT.md</a><br> > [3] <a href="https://github.com/sugarlabs/sugar-docs/blob/master/src/contributing.md#making-commits" rel="noreferrer" target="_blank">https://github.com/sugarlabs/s<wbr>ugar-docs/blob/master/src/cont<wbr>ributing.md#making-commits</a><br> > [4] <a href="http://bugs.sugarlabs.org/" rel="noreferrer" target="_blank">http://bugs.sugarlabs.org/</a><br> > [5] <a href="https://github.com/sugarlabs/sugar-docs/blob/master/src/desktop-activity.md#coding-standards" rel="noreferrer" target="_blank">https://github.com/sugarlabs/s<wbr>ugar-docs/blob/master/src/desk<wbr>top-activity.md#coding-standar<wbr>ds</a><br> > [6] <a href="https://wiki.sugarlabs.org/go/Development_Team/Release/Modules#Fructose" rel="noreferrer" target="_blank">https://wiki.sugarlabs.org/go/<wbr>Development_Team/Release/Modul<wbr>es#Fructose</a><br> > [7] <a href="http://quozl.netrek.org/" rel="noreferrer" target="_blank">http://quozl.netrek.org/</a><br> <div class="m_6685955816184410063HOEnZb"><div class="m_6685955816184410063h5"><br> --<br> James Cameron<br> <a href="http://quozl.netrek.org/" rel="noreferrer" target="_blank">http://quozl.netrek.org/</a><br> </div></div></blockquote></div><br></div></div>