<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Fri, Apr 21, 2017 at 1:28 PM Dan Kenigsberg <<a href="mailto:danken@redhat.com">danken@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Fri, Apr 21, 2017 at 1:04 PM, Nir Soffer <<a href="mailto:nsoffer@redhat.com" target="_blank">nsoffer@redhat.com</a>> wrote:<br>
> On Fri, Apr 21, 2017 at 11:19 AM Dan Kenigsberg <<a href="mailto:danken@redhat.com" target="_blank">danken@redhat.com</a>> wrote:<br>
>><br>
>> As we all know, Python is dynamically typed, which requires good<br>
>> coverage by unit tests.<br>
>><br>
>> As we all know, Vdsm unit tests' coverage is not good.<br>
>><br>
>> We need all the help we can get in order to avoid typos leading to<br>
>> bugs. Nir's `make pylint-diff` would give some.<br>
><br>
><br>
> We can take this patch now, since it does not break the build on errors.<br>
><br>
> It is useful as is so people can run make pylint locally and fix their<br>
> errors.<br>
><br>
>><br>
>> In order to have it<br>
>> merged, I have posted<br>
>><br>
>> <a href="https://gerrit.ovirt.org/#/q/status:open+project:vdsm+branch:master+topic:pylint+owner:danken" rel="noreferrer" target="_blank">https://gerrit.ovirt.org/#/q/status:open+project:vdsm+branch:master+topic:pylint+owner:danken</a><br>
>> to silence existing pylint errors. I hope I did not silence any<br>
>> serious ones; please review this topic as soon as possible.<br>
><br>
><br>
> Thanks for this great work!<br>
><br>
> I think the last patch is too big, and there are some storage issues that we<br>
> can fix<br>
> now. Can you split by vertical? I would like to take over the the storage<br>
> part.<br>
<br>
Please do. </blockquote><div><br></div><div>I splitted the patches to:</div><div><br></div><div>- <a href="https://gerrit.ovirt.org/75728">https://gerrit.ovirt.org/75728</a> pylint: brutally silence two network-related errors</div><div>- <a href="https://gerrit.ovirt.org/75730">https://gerrit.ovirt.org/75730</a> pylint: Silence pylint errors in gluster</div><div>- <a href="https://gerrit.ovirt.org/75748">https://gerrit.ovirt.org/75748</a> pylint: Silence pylint errors in infra</div><div>- <a href="https://gerrit.ovirt.org/75749">https://gerrit.ovirt.org/75749</a> pylint: Silence pylint errors in virt</div><div>- <a href="https://gerrit.ovirt.org/75750">https://gerrit.ovirt.org/75750</a> pylint: Silence pylint errors in storage<br></div><div><br></div><div>I'll handle the storage topic, hopefully Sahina, Piotr, Francesco and Edy will </div><div>care for the others.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Particularly, please look at the true error introduced by<br>
I8e9bde2a0ad899ae56a18b4c7d6b405360d295f2 (there are two calls to<br>
_deleteImage missing the new discard value).<br></blockquote><div><br></div><div>We know about this, the code calling these is not used for years. The real</div><div>fix is to remove the unused code. If we will not have time to remove the code</div><div>I'll leave the pylint comments.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I hope Piotr and Francesco can take over theirs, as the time I had to<br>
invest in this is basically up (hence the gobbling of assorted fixes<br>
in one ugly patch).<br>
<br>
><br>
> I sent this patch making the storage/monitor.py pylint clean:<br>
> <a href="https://gerrit.ovirt.org/75736" rel="noreferrer" target="_blank">https://gerrit.ovirt.org/75736</a><br>
><br>
> Nir<br>
</blockquote></div></div>