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

Thu Aug 1 04:06:32 UTC 2013

On Wed, 2013-07-31 at 16:43 -0400, Ade Lee wrote:
> 1. There is a super long line in the compose script.  Please break that
> into multiple lines.
> 
> +   echo -e "If the reported issues are false positives, please mark them ignored  \nin the configuration file '$PKI_PWD/$PKI_DIR/dogtag.pylintrc' \nand build again."
> 
> 2. The logic for determining whether or not to stop the script based on the return value is incorrect.
> 
> +#OUTPUT STATUS CODE
> +#       Pylint should leave with following status code:
> +#           * 0 if everything went fine
> +#           * 1 if a fatal message was issued
> +#           * 2 if an error message was issued
> +#           * 4 if a warning message was issued
> +#           * 8 if a refactor message was issued
> +#           * 16 if a convention message was issued
> +#           * 32 on usage error
> +#
> +#       status 1 to 16 will be bit-ORed so you can know which different categories has been issued by analysing pylint output status code
> +
> +result=0
> +if [ $(($status&1)) -eq 1 ] || [ $(($status&2)) -eq 2 ] || [ $(($status&4)) -eq 4 ]
> +then
> +  result=1
> +fi
> 
> what happens if there is an error and a warning?  Then the return value
> will be 6.  You need to decompose the error status and determine which
> bits were set.
> 
This might help:

How to check if the i-th bit is set in an integer
To check if a the i-th bit of a number n is set, we can see if ( n & 1
<< i ) is non-zero. To be precise if the bit is set, then the result
would be 2i. This is because in the number 2i there is only 1-bit and
that is the i-th bit, if it's ANDing with n results in a non-zero number
then the i-th bit is set in n.

> Ade
> 
> 
> On Mon, 2013-07-29 at 11:56 -0400, Abhishek Koneru wrote:
> > 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
> > 
> 
> 
> _______________________________________________
> Pki-devel mailing list
> Pki-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel