| Message ID | 20180804194734.12577-1-nicolas.iooss@m4x.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fix some issues found by flake8 | expand |
On Sat, Aug 4, 2018 at 12:47 PM, Nicolas Iooss <nicolas.iooss@m4x.org> wrote: > Hi, > > I have been working on a script which uses flake8 to discover issues in > Python code. This led me to discover several issues which are fixed by > these patches. Distribution maintainers might be interested in > backporting some of them (at least patches 5 and 10 and probably the > ones which fix usage of undefined variables). > > As Travis-CI is experiencing issues today (it fails to launch new > builds), I have not been able to test the integration of my script with > Travis-CI yet. Once it works again I will submit this script too. > > Thanks, > Nicolas > > --- > Here comes the description generated by "git format-patch --cover": > > Nicolas Iooss (13): > libselinux: fix flake8 warnings in SWIG-generated code > python/sepolgen: do not import twice the modules > python/sepolgen: return NotImplemented instead of raising it > python/sepolicy: drop unused CheckPolicyType > python/sepolicy: use lowercase variable name > python/sepolgen: fix refpolicy parsing of "permissive" > python/sepolgen: silence linter warning about has_key > This is the only one I don't particularly like: > python/sepolgen: comment buggy code > If we choose to comment out it out, a block comment explaining why would be helpful or just delete it if it is dead code. python/sepolgen: use self when accessing members in FilesystemUse > python/sepolicy: fix "procotol" misspelling > python/sepolicy: use variables which exists in the gui.py > python/sepolicy: do not import sepolicy.generate.DAEMON twice > python/sepolicy: do not import types > > libselinux/src/selinuxswig_python.i | 5 +++-- > python/sepolgen/src/sepolgen/interfaces.py | 10 +++++----- > python/sepolgen/src/sepolgen/refparser.py | 4 ++-- > python/sepolgen/src/sepolgen/refpolicy.py | 6 +++--- > python/sepolgen/src/sepolgen/util.py | 6 +----- > python/sepolicy/sepolicy.py | 12 +----------- > python/sepolicy/sepolicy/generate.py | 1 - > python/sepolicy/sepolicy/gui.py | 12 ++++++------ > 8 files changed, 21 insertions(+), 35 deletions(-) > > -- > 2.18.0 > > _______________________________________________ > Selinux mailing list > Selinux@tycho.nsa.gov > To unsubscribe, send email to Selinux-leave@tycho.nsa.gov. > To get help, send an email containing "help" to > Selinux-request@tycho.nsa.gov. > <div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Aug 4, 2018 at 12:47 PM, Nicolas Iooss <span dir="ltr"><<a href="mailto:nicolas.iooss@m4x.org" target="_blank">nicolas.iooss@m4x.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br> <br> I have been working on a script which uses flake8 to discover issues in<br> Python code. This led me to discover several issues which are fixed by<br> these patches. Distribution maintainers might be interested in<br> backporting some of them (at least patches 5 and 10 and probably the<br> ones which fix usage of undefined variables).<br> <br> As Travis-CI is experiencing issues today (it fails to launch new<br> builds), I have not been able to test the integration of my script with<br> Travis-CI yet. Once it works again I will submit this script too.<br> <br> Thanks,<br> Nicolas<br> <br> ---<br> Here comes the description generated by "git format-patch --cover":<br> <br> Nicolas Iooss (13):<br> libselinux: fix flake8 warnings in SWIG-generated code<br> python/sepolgen: do not import twice the modules<br> python/sepolgen: return NotImplemented instead of raising it<br> python/sepolicy: drop unused CheckPolicyType<br> python/sepolicy: use lowercase variable name<br> python/sepolgen: fix refpolicy parsing of "permissive"<br> </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> python/sepolgen: silence linter warning about has_key<br> </blockquote><div><br></div><div>This is the only one I don't particularly like: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> python/sepolgen: comment buggy code<br></blockquote><div><br></div><div>If we choose to comment out it out, a block comment explaining why would be</div><div>helpful or just delete it if it is dead code.</div><div><br></div><div> python/sepolgen: use self when accessing members in FilesystemUse</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> python/sepolicy: fix "procotol" misspelling<br> python/sepolicy: use variables which exists in the gui.py<br> python/sepolicy: do not import sepolicy.generate.DAEMON twice<br> python/sepolicy: do not import types<br> <br> libselinux/src/selinuxswig_<wbr>python.i | 5 +++--<br> python/sepolgen/src/sepolgen/<wbr>interfaces.py | 10 +++++-----<br> python/sepolgen/src/sepolgen/<wbr>refparser.py | 4 ++--<br> python/sepolgen/src/sepolgen/<wbr>refpolicy.py | 6 +++---<br> python/sepolgen/src/sepolgen/<wbr>util.py | 6 +-----<br> python/sepolicy/sepolicy.py | 12 +-----------<br> python/sepolicy/sepolicy/<wbr>generate.py | 1 -<br> python/sepolicy/sepolicy/gui.<wbr>py | 12 ++++++------<br> 8 files changed, 21 insertions(+), 35 deletions(-)<br> <span class="HOEnZb"><font color="#888888"><br> -- <br> 2.18.0<br> <br> ______________________________<wbr>_________________<br> Selinux mailing list<br> <a href="mailto:Selinux@tycho.nsa.gov">Selinux@tycho.nsa.gov</a><br> To unsubscribe, send email to <a href="mailto:Selinux-leave@tycho.nsa.gov">Selinux-leave@tycho.nsa.gov</a>.<br> To get help, send an email containing "help" to <a href="mailto:Selinux-request@tycho.nsa.gov">Selinux-request@tycho.nsa.gov</a>.<br> </font></span></blockquote></div><br></div></div>
On Mon, Aug 6, 2018 at 5:05 PM, William Roberts <bill.c.roberts@gmail.com> wrote: > > On Sat, Aug 4, 2018 at 12:47 PM, Nicolas Iooss <nicolas.iooss@m4x.org> > wrote: >> >> Hi, >> >> I have been working on a script which uses flake8 to discover issues in >> Python code. This led me to discover several issues which are fixed by >> these patches. Distribution maintainers might be interested in >> backporting some of them (at least patches 5 and 10 and probably the >> ones which fix usage of undefined variables). >> >> As Travis-CI is experiencing issues today (it fails to launch new >> builds), I have not been able to test the integration of my script with >> Travis-CI yet. Once it works again I will submit this script too. >> >> Thanks, >> Nicolas >> >> --- >> Here comes the description generated by "git format-patch --cover": >> >> Nicolas Iooss (13): >> libselinux: fix flake8 warnings in SWIG-generated code >> python/sepolgen: do not import twice the modules >> python/sepolgen: return NotImplemented instead of raising it >> python/sepolicy: drop unused CheckPolicyType >> python/sepolicy: use lowercase variable name >> python/sepolgen: fix refpolicy parsing of "permissive" >> >> python/sepolgen: silence linter warning about has_key > > > This is the only one I don't particularly like: >> >> python/sepolgen: comment buggy code > > > If we choose to comment out it out, a block comment explaining why would be > helpful or just delete it if it is dead code. It feels like this code could be useful if an interface parameter ("$1") is used as a permission, which is why I have not deleted it. I suggest adding the following comment: # This function currently ignores parameters which are used in permission. # The following code has been present for a long time and contains two # syntax errors (__param_insert takes 4 arguments and PERM is not defined) # which proves that it is actually dead code. #for perm in av.perms: # if access.is_idparam(perm): # if __param_insert(perm, PERM) == 1: # ret = 1 Does this suit you? By the way, I do not have a strong opinion about commenting or deleting it, so I am fine if we choose to remove this code instead. Thanks for your review, Nicolas
On Mon, Aug 6, 2018 at 1:26 PM, Nicolas Iooss <nicolas.iooss@m4x.org> wrote: > On Mon, Aug 6, 2018 at 5:05 PM, William Roberts > <bill.c.roberts@gmail.com> wrote: > > > > On Sat, Aug 4, 2018 at 12:47 PM, Nicolas Iooss <nicolas.iooss@m4x.org> > > wrote: > >> > >> Hi, > >> > >> I have been working on a script which uses flake8 to discover issues in > >> Python code. This led me to discover several issues which are fixed by > >> these patches. Distribution maintainers might be interested in > >> backporting some of them (at least patches 5 and 10 and probably the > >> ones which fix usage of undefined variables). > >> > >> As Travis-CI is experiencing issues today (it fails to launch new > >> builds), I have not been able to test the integration of my script with > >> Travis-CI yet. Once it works again I will submit this script too. > >> > >> Thanks, > >> Nicolas > >> > >> --- > >> Here comes the description generated by "git format-patch --cover": > >> > >> Nicolas Iooss (13): > >> libselinux: fix flake8 warnings in SWIG-generated code > >> python/sepolgen: do not import twice the modules > >> python/sepolgen: return NotImplemented instead of raising it > >> python/sepolicy: drop unused CheckPolicyType > >> python/sepolicy: use lowercase variable name > >> python/sepolgen: fix refpolicy parsing of "permissive" > >> > >> python/sepolgen: silence linter warning about has_key > > > > > > This is the only one I don't particularly like: > >> > >> python/sepolgen: comment buggy code > > > > > > If we choose to comment out it out, a block comment explaining why would > be > > helpful or just delete it if it is dead code. > > It feels like this code could be useful if an interface parameter > ("$1") is used as a permission, which is why I have not deleted it. I > suggest adding the following comment: > > # This function currently ignores parameters which are used in > permission. > # The following code has been present for a long time and contains two > # syntax errors (__param_insert takes 4 arguments and PERM is not > defined) > # which proves that it is actually dead code. > #for perm in av.perms: > # if access.is_idparam(perm): > # if __param_insert(perm, PERM) == 1: > # ret = 1 > > Does this suit you? By the way, I do not have a strong opinion about > commenting or deleting it, so I am fine if we choose to remove this > code instead. > I'm all for deleting it. I think we should just do that. > > Thanks for your review, > Nicolas > > <div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Aug 6, 2018 at 1:26 PM, Nicolas Iooss <span dir="ltr"><<a href="mailto:nicolas.iooss@m4x.org" target="_blank">nicolas.iooss@m4x.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Mon, Aug 6, 2018 at 5:05 PM, William Roberts<br> <<a href="mailto:bill.c.roberts@gmail.com">bill.c.roberts@gmail.com</a>> wrote:<br> ><br> > On Sat, Aug 4, 2018 at 12:47 PM, Nicolas Iooss <<a href="mailto:nicolas.iooss@m4x.org">nicolas.iooss@m4x.org</a>><br> > wrote:<br> >><br> >> Hi,<br> >><br> >> I have been working on a script which uses flake8 to discover issues in<br> >> Python code. This led me to discover several issues which are fixed by<br> >> these patches. Distribution maintainers might be interested in<br> >> backporting some of them (at least patches 5 and 10 and probably the<br> >> ones which fix usage of undefined variables).<br> >><br> >> As Travis-CI is experiencing issues today (it fails to launch new<br> >> builds), I have not been able to test the integration of my script with<br> >> Travis-CI yet. Once it works again I will submit this script too.<br> >><br> >> Thanks,<br> >> Nicolas<br> >><br> >> ---<br> >> Here comes the description generated by "git format-patch --cover":<br> >><br> >> Nicolas Iooss (13):<br> >> libselinux: fix flake8 warnings in SWIG-generated code<br> >> python/sepolgen: do not import twice the modules<br> >> python/sepolgen: return NotImplemented instead of raising it<br> >> python/sepolicy: drop unused CheckPolicyType<br> >> python/sepolicy: use lowercase variable name<br> >> python/sepolgen: fix refpolicy parsing of "permissive"<br> >><br> >> python/sepolgen: silence linter warning about has_key<br> ><br> ><br> > This is the only one I don't particularly like:<br> >><br> >> python/sepolgen: comment buggy code<br> ><br> ><br> > If we choose to comment out it out, a block comment explaining why would be<br> > helpful or just delete it if it is dead code.<br> <br> </div></div>It feels like this code could be useful if an interface parameter<br> ("$1") is used as a permission, which is why I have not deleted it. I<br> suggest adding the following comment:<br> <br> # This function currently ignores parameters which are used in permission.<br> # The following code has been present for a long time and contains two<br> # syntax errors (__param_insert takes 4 arguments and PERM is not defined)<br> # which proves that it is actually dead code.<br> #for perm in av.perms:<br> # if access.is_idparam(perm):<br> # if __param_insert(perm, PERM) == 1:<br> # ret = 1<br> <br> Does this suit you? By the way, I do not have a strong opinion about<br> commenting or deleting it, so I am fine if we choose to remove this<br> code instead.<br></blockquote><div><br></div><div>I'm all for deleting it. I think we should just do that.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br> Thanks for your review,<br> Nicolas<br> <br> </blockquote></div><br></div></div>