[Pki-devel] [PATCH] 71 Patch for ticket 316- Adding pylint to the build process

[Abhishek Koneru]

A few questions/ comments:

1. In your build, you create a pylintscan directory within the source
tree under pki.  I would prefer this to take place in the build area so
that it does not clutter up your source tree.  The pylintscan is a build
artifact.

-- Sure can be moved. I can delete the folder once the scan is done. since the scan actually takes place before
   the build. (As per answer 2 - can i still keep pylintscan in pki?)

2. Do you need to remove the directory prior to each run?

-- Not required. I will remove it in the script. Will just leave the pylint-report.
   Is it ok to put the report in the root directory of the code tree - pki?

3. What parts of the pylintrc are not default? ie. which parts did you
change?

-- The disable-ids property. (Will not disable conventions [C] and refactor[R].)
-- Max lines - from 80 to 100 
-- Removed 'string' package from deprecated modules.
-- Allowing variables to be named 'rv'. (property - good-names)
-- Set property - include ids = yes.

4. I noticed that you added JSONEncoder to the pylintrc as a class for
which private members should not be checked.  Does that allow you to
eliminate the comment to # pylint: disable-msg=E0202 in encoder.py?

-- Forgot to squash a few final changes made to pylintrc. it was actually not included in pylintrc.

5. Can you explain the rationale for ignoring  W0511 and W0105?
 W0105 (pointless-string-statement):
       String statement has no effect Used when a string is used as a statement (which of course has no effect). This is a particular case of W0104 with its own message so you can easily disable it if you’re   #       using those strings as documentation, instead of comments.
 W0511 (fixme): Used when a warning note as FIXME or XXX is detected.

-- Will add the same to the comments for disabled ids.

6. I would prefer to not ignore Convention, refactor etc. and to have
these show up in the pylint reports.  Based on the return code for
pylint, you can have the build fail only if the bits for errors and
warnings are set.

-- Can be done. will make the change.

7. The pylint command line seems to have all the python executables
listed -- is this required?  It means having to update this list every
time an executable is added, which is a step that can easily be missed.
Can you just point the scan to the top level directory?  

 -- Yes. Did not notice that Endi added __init__.py in all directories. Will change that.

8.  Similarly to the point above, is there a way to scan the source tree
to find all the python code?

 -- Yes it can be done. But since the number of source files is very small, i used this approach of copying
    the directories directly instead of scanning all the code.

On Mon, 2013-07-22 at 21:32 -0400, Abhishek Koneru wrote:
> Please review the patch which adds a script and also the pylint
> configuration file to the code tree. The script is called in the compose
> script for core packages before the actual packaging is done. If any
> errors or warnings are reported by pylint, the build fails.
> 
> I did not add pylint as part of build-requires in the spec file for
> pki-core, but have put a check in the script to bypass trying to scan if
> pylint is not installed but with a comment stating the same in the log.
> 
> --Abhishek
> _______________________________________________
> Pki-devel mailing list
> Pki-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel