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

[Abhishek Koneru]

PFA the patch  which addressed all comments mentioned below.
Raised a ticket for question number 8.

--Abhishek

On Fri, 2013-07-26 at 16:13 -0400, Abhishek Koneru wrote:
> 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
> 
> 
> 
> _______________________________________________
> Pki-devel mailing list
> Pki-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: pki-akoneru-0071-2-Adding-pylint-scan-to-the-build.patch
Type: text/x-patch
Size: 12467 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/pki-devel/attachments/20130729/7e4f3d1b/attachment.bin>