[JENKINS-46035] "Hardcoded Script Checker - Threshold": Don't count comment lines or empty lines - Jenkins Jira

XML

Word

Printable

Details

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

Similar Issues:

Show

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

Descending order - Click to sort in ascending order

Hide

Permalink

Victor Martinez added a comment - 2017-08-11 12:57

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

~~JENKINS-46146~~

Show

Victor Martinez added a comment - 2017-08-11 12:57 Martin Jost , let's follow up the comment lines ticket in the below cloned one: JENKINS-46146

Hide

Permalink

Victor Martinez added a comment - 2017-08-10 19:32

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

Show

Victor Martinez added a comment - 2017-08-10 19:32 Ignoring empty lines will be part of the upcoming release: 0.11.0

Hide

Permalink

SCM/JIRA link daemon added a comment - 2017-08-10 19:26

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

Show

SCM/JIRA link daemon added a comment - 2017-08-10 19:26 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

Permalink

Martin Jost added a comment - 2017-08-10 13:44

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

Martin Jost added a comment - 2017-08-10 13:44 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

Permalink

SCM/JIRA link daemon added a comment - 2017-08-09 16:32

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/JIRA link daemon added a comment - 2017-08-09 16:32 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

Permalink

Victor Martinez added a comment - 2017-08-09 08:25

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

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

Victor Martinez added a comment - 2017-08-09 08:25 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

People

Assignee:: Victor Martinez

Reporter:: Martin Jost

Votes:: 0 Vote for this issue

Watchers:: 3 Start watching this issue

Dates

Created:: 2017-08-07 16:46

Updated:: 2017-08-16 22:00

Resolved:: 2017-08-10 19:32