On Sun, Aug 19, 2018 at 1:53 AM, Nicolas Iooss <nicolas.iooss@xxxxxxx> wrote:
On Sat, Aug 18, 2018 at 8:43 PM William Roberts
<bill.c.roberts@xxxxxxxxx> wrote:
>
> Im assuming with your attention on the python side of the house we're going to see a lot of
> formatting change patches heading the mailing list. I don't have any problems with them.
>
> Are you using some formatter for these, if so which one is it? Is it flake8 still?
I did not use a "formatter" here (I do not consider 2to3 to be a
formatter). I only looked at pep8 and pylint warnings and modified the
code with search-and-replace commands on the file (using regular
expressions). I usually do not like large changes which reindent a
file or add spaces, because it increases the amount of work needed to
backport a later bugfix to released versions, for package maintainers.
Nevertheless when the modified file is not used "in production" (only
when debugging issues or when testing things), as is the case here, I
prefer cleaning up the code.
This seems reasonable. Clean up flake8 errors while ensuring we don't introduce
new ones.
> We should probably document that patches should be sent formatted, ie if a patch introduces a delta
> after applying the patch and running <formatter>, it's an issue.
I upstreamed scripts/run-flake8 with this in mind ;) (and this is why
I added it to Travis-CI tests too). And one of the reasons that led me
to disable many warnings was to preserve some flexibility in order not
to make submitting patches too hard for new contributors.
In the next days, I plan to send some more patches to fix some
formatting issues I consider important (for example removing
semicolons at the end of Python statements), but I will not send
patches which modify the spaces around operators in files where there
could be bugs which would need to be backported.
When I review these I am literally just applying the patches, running the test and
making sure that flake8 errors go down. I'm thinking for these, since you have
we have this integrated in the build, send the patches, but if no one responds
id be ok with you applying them. Albeit this not 100% in line with agreed practices
these patches are just mostly large noise changes. Does anyone rebuke this?
As always, thanks for your review!
Nicolas
PS: I am also playing with clang's static analyzer and I am currently
testing a CircleCI configuration which runs it on every push (using
scripts/run-scan-build) and publishes the results as HTML build
artifacts. I will submit a proper patch for this once it is stable
enough and once all the most important issues reported by the analyzer
are fixed.
Oh the HTML will be interesting. I have scan-build integrated in my Travis
for other projects. Its nice.
_______________________________________________ Selinux mailing list Selinux@xxxxxxxxxxxxx To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx. To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.
