Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-46035

"Hardcoded Script Checker - Threshold": Don't count comment lines or empty lines

    Details

    • Type: Improvement
    • Status: Closed (View Workflow)
    • Priority: Minor
    • Resolution: Fixed
    • Component/s: jenkinslint-plugin
    • Labels:
    • Environment:
      Jenkins 2.71 running using JDK1.8_141 on RH6 Linux, no container
      jenkinslint 0.10.1
    • Similar Issues:

      Description

      It seems the

        Hardcoded Script Checker - Threshold

      counts at least comment lines (and probably also empty lines or lines just containing whitespace)

      It would be nice, to not count these. We sometimes have comment blocks before one or two lines of script calling. This now forces us, to set a rather big threshold.

      And as an extra a help entry on the configuration page would be nice for this entry.

        Attachments

          Activity

          Hide
          v2v Victor Martinez added a comment -

          You are right, It's not a script parser at the moment, just counting the number of lines purely.

          Blank lines shouldn't be part of the total number of lines, but I'm not sure about the comments, in my opinion commented code should be also part of this lint.

          Although I do understand the commented header, as you can see below, could be ignored.

           

          #!/bin/bash
          # DESC: this is my script to build my awesome program
          # INPUT: BUILD_ID, TARGET, PLATFORM
          # OUTPUT: awesome.zip
          #
          
          . ./properties.properties
          ./build.sh $TARGET $PLATFORM $BUILD_ID
          #if [ $? -gt 0 ] ; then 
          #  echo "DEBUG=true" > status.properties
          #fi
          echo "DEBUG=false" > status.properties

           

          My ideal use case would be:

          • Script the logic, helper and information in the script
          • Enable a --help flag to prompt the information about how to use it
          • Upload the script to the SCM with a semantic name about what it does.
          • And describe the aim of it in the job description. 

          Something like:

          1) build_platform.sh

           

          #!/bin/bash
          # DESC: this is my script to build my awesome program
          # INPUT: BUILD_ID, TARGET, PLATFORM
          # OUTPUT: awesome.zip
          # OPTIONS: --build --help
          
          if [ "$1" == "--help" ]; then
            echo "Usage: `basename $0` [--build TARGET PLATFORM BUILD_ID | --help] "
            exit 0
          fi
          ...
          ...
          echo "DEBUG=false" > status.properties

           

          2)

           

          job ('myjob') {
              description('This Job will generate an awesome zip based on the Platform and Target')
              parameters {
                  stringParam('TARGET', 'default', 'Build target')
                  stringParam('PLATFORM', 'default', 'Build platform')
              }
              scm {
                   github('test/awesome.git')
              }
              steps {
                  shell("./build.sh $TARGET $PLATFORM $BUILD_ID")
              }
          }

           

           

          Rather than:

          job ('myjob') {
              description('This Job will generate an awesome zip based on the Platform and Target')
              parameters {
                  stringParam('TARGET', 'default', 'Build target')
                  stringParam('PLATFORM', 'default', 'Build platform')
              }
              scm {
                   github('test/awesome.git')
              }
              steps {
                  shell """
                    #!/bin/bash
                    # DESC: this is my script to build my awesome program
                    # INPUT: BUILD_ID, TARGET, PLATFORM
                    # OUTPUT: awesome.zip
                    #
          
                    . ./properties.properties
                    ./build.sh $TARGET $PLATFORM $BUILD_ID
                    #if [ $? -gt 0 ] ; then 
                    #  echo "DEBUG=true" > status.properties
                    #fi
                    echo "DEBUG=false" > status.properties
                  """
              }
          }

           

           

          What do you think? Does it make sense to only care about the non blank lines?

          Cheers 

          Show
          v2v Victor Martinez added a comment - You are right, It's not a script parser at the moment, just counting the number of lines purely. Blank lines shouldn't be part of the total number of lines, but I'm not sure about the comments, in my opinion commented code should be also part of this lint. Although I do understand the commented header, as you can see below, could be ignored.   #!/bin/bash # DESC: this is my script to build my awesome program # INPUT: BUILD_ID, TARGET, PLATFORM # OUTPUT: awesome.zip # . ./properties.properties ./build.sh $TARGET $PLATFORM $BUILD_ID # if [ $? -gt 0 ] ; then # echo "DEBUG= true " > status.properties #fi echo "DEBUG= false " > status.properties   My ideal use case would be: Script the logic, helper and information in the script Enable a --help flag to prompt the information about how to use it Upload the script to the SCM with a semantic name about what it does. And describe the aim of it in the job description.  Something like: 1) build_platform.sh   #!/bin/bash # DESC: this is my script to build my awesome program # INPUT: BUILD_ID, TARGET, PLATFORM # OUTPUT: awesome.zip # OPTIONS: --build --help if [ "$1" == "--help" ]; then echo "Usage: `basename $0` [--build TARGET PLATFORM BUILD_ID | --help] " exit 0 fi ... ... echo "DEBUG= false " > status.properties   2)   job ( 'myjob' ) { description( 'This Job will generate an awesome zip based on the Platform and Target' ) parameters { stringParam( 'TARGET' , ' default ' , 'Build target' ) stringParam( 'PLATFORM' , ' default ' , 'Build platform' ) } scm { github( 'test/awesome.git' ) } steps { shell( "./build.sh $TARGET $PLATFORM $BUILD_ID" ) } }     Rather than: job ( 'myjob' ) { description( 'This Job will generate an awesome zip based on the Platform and Target' ) parameters { stringParam( 'TARGET' , ' default ' , 'Build target' ) stringParam( 'PLATFORM' , ' default ' , 'Build platform' ) } scm { github( 'test/awesome.git' ) } steps { shell """           #!/bin/bash           # DESC: this is my script to build my awesome program           # INPUT: BUILD_ID, TARGET, PLATFORM           # OUTPUT: awesome.zip           #           . ./properties.properties           ./build.sh $TARGET $PLATFORM $BUILD_ID           # if [ $? -gt 0 ] ; then           # echo "DEBUG= true " > status.properties           #fi           echo "DEBUG= false " > status.properties """ } }     What do you think? Does it make sense to only care about the non blank lines? Cheers 
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Victor Martinez
          Path:
          src/main/java/org/jenkins/ci/plugins/jenkinslint/check/HardcodedScriptChecker.java
          src/test/java/org/jenkins/ci/plugins/jenkinslint/check/HardcodedScriptCheckerTestCase.java
          http://jenkins-ci.org/commit/jenkinslint-plugin/5d985b2bcb22b2a3b04978d46fa12f10c91297e6
          Log:
          JENKINS-46035 Empty and Blank lines should be ignored

          Change-Id: I0baa892f0c9e21852b5a423d2189f79fbc119985

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Victor Martinez Path: src/main/java/org/jenkins/ci/plugins/jenkinslint/check/HardcodedScriptChecker.java src/test/java/org/jenkins/ci/plugins/jenkinslint/check/HardcodedScriptCheckerTestCase.java http://jenkins-ci.org/commit/jenkinslint-plugin/5d985b2bcb22b2a3b04978d46fa12f10c91297e6 Log: JENKINS-46035 Empty and Blank lines should be ignored Change-Id: I0baa892f0c9e21852b5a423d2189f79fbc119985
          Hide
          martinjost Martin Jost added a comment -

          Surely the non-empty lines are a first step.
          What we actually have is something like e.g.

          #!/var/.../tools/bin/bash -x

          # set to "true" to get additional debug output
          # verbose=true

          # SCT     normal build for ci usage
          # xMT     x Module test
          # yMT     y Module test
          # BM      Benchmarking
          # Default Get it from the job name
          # buildVariant="SCT"

          ##################################################################
          export scriptDir=/a/b/ciScripts/bin
          #export scriptDir=/home/me/build/scripts/scripts/bin
          #export scriptDir=/home/me/build/scripts/scriptsJustFixes/bin
          source ${scriptDir}/FOOBAR-build.sh

          And "obviously" "FOOBAR-build.sh" is in SVN.

          Show
          martinjost Martin Jost added a comment - Surely the non-empty lines are a first step. What we actually have is something like e.g. #!/var/.../tools/bin/bash -x # set to "true" to get additional debug output # verbose=true # SCT     normal build for ci usage # xMT     x Module test # yMT     y Module test # BM      Benchmarking # Default Get it from the job name # buildVariant="SCT" ################################################################## export scriptDir=/a/b/ciScripts/bin #export scriptDir=/home/ me /build/scripts/scripts/bin #export scriptDir=/home/ me /build/scripts/scriptsJustFixes/bin source ${scriptDir}/FOOBAR-build.sh And "obviously" " FOOBAR-build.sh " is in SVN.
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Victor Martinez
          Path:
          src/main/java/org/jenkins/ci/plugins/jenkinslint/check/HardcodedScriptChecker.java
          src/test/java/org/jenkins/ci/plugins/jenkinslint/check/HardcodedScriptCheckerTestCase.java
          http://jenkins-ci.org/commit/jenkinslint-plugin/6458bd62d06a61b230d9456ac804b6236660a9f5
          Log:
          JENKINS-46035 Don't count empty lines (#27)

          Change-Id: I0baa892f0c9e21852b5a423d2189f79fbc119985

          • Refactored a bit and removed duplicated hardcoded strings
          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Victor Martinez Path: src/main/java/org/jenkins/ci/plugins/jenkinslint/check/HardcodedScriptChecker.java src/test/java/org/jenkins/ci/plugins/jenkinslint/check/HardcodedScriptCheckerTestCase.java http://jenkins-ci.org/commit/jenkinslint-plugin/6458bd62d06a61b230d9456ac804b6236660a9f5 Log: JENKINS-46035 Don't count empty lines (#27) JENKINS-46035 Empty and Blank lines should be ignored Change-Id: I0baa892f0c9e21852b5a423d2189f79fbc119985 Refactored a bit and removed duplicated hardcoded strings
          Hide
          v2v Victor Martinez added a comment -

          Ignoring empty lines will be part of the upcoming release: 0.11.0

          Show
          v2v Victor Martinez added a comment - Ignoring empty lines will be part of the upcoming release: 0.11.0
          Hide
          v2v Victor Martinez added a comment -

          Martin Jost, let's follow up the comment lines ticket in the below cloned one:

          Show
          v2v Victor Martinez added a comment - Martin Jost , let's follow up the comment lines ticket in the below cloned one: JENKINS-46146

            People

            • Assignee:
              v2v Victor Martinez
              Reporter:
              martinjost Martin Jost
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: