<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, May 8, 2013 at 5:31 AM, Daniel Narvaez <span dir="ltr"><<a href="mailto:dwnarvaez@gmail.com" target="_blank">dwnarvaez@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="im">On 8 May 2013 06:02, William Orr <span dir="ltr"><<a href="mailto:will@worrbase.com" target="_blank">will@worrbase.com</a>></span> wrote:<br>
</div><div class="gmail_extra"><div class="gmail_quote"><div class="im"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
I've done some initial work on both sugar and sugar-toolkit-gtk3 with respect to PEP8. I've also added support for PEP8 in make check for both packages.<br>
<br>
Pull requests are here:<br>
<a href="https://github.com/sugarlabs/sugar-toolkit-gtk3/pull/25" target="_blank">https://github.com/sugarlabs/<u></u>sugar-toolkit-gtk3/pull/25</a><br>
<a href="https://github.com/sugarlabs/sugar/pull/49" target="_blank">https://github.com/sugarlabs/<u></u>sugar/pull/49</a><br>
<br>
Let me know if I need to make any changes.<br></blockquote><div><br></div></div><div>Awesome! Thanks for working on this!<br></div></div></div></div></blockquote><div><br>+1<br> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>I didn't go through all the changes yet, but I have one general concern. You have disabled E501 (line too long), which would be fine as an intermediate step, though you seem to be regressing line length when fixing the other issues. Perhaps it make sense to do the E501 changes together so that we avoid regressions?<br>

<br>If you end up reworking the patches it might also make sense to split them up a bit more (even arbitrarily) just to review/land more gradually and diminish a bit the risk of conflicts etc. I don't expect the review to be particularly complicated, though there are a few odd cases with line continuations that we will need to check/discuss/tweak.<br>
</div></div></div></div></blockquote><div><br></div><div>Also, there are a few non-pep8 fixes mixed in as well, e.g., replacing type() == with isinstance()... a needed change but perhaps the kind of thing that could go in a separate patch.<br>
<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>
</div></div></div></div>
<br>_______________________________________________<br>
Sugar-devel mailing list<br>
<a href="mailto:Sugar-devel@lists.sugarlabs.org">Sugar-devel@lists.sugarlabs.org</a><br>
<a href="http://lists.sugarlabs.org/listinfo/sugar-devel" target="_blank">http://lists.sugarlabs.org/listinfo/sugar-devel</a><br>
<br></blockquote></div><br><br clear="all"><br>-- <br>Walter Bender<br>Sugar Labs<br><a href="http://www.sugarlabs.org">http://www.sugarlabs.org</a><br>
</div></div>