awslabs / linter-rules-for-nextflow

Linter rules for Nextflow DSL scripts
Apache License 2.0
28 stars 1 forks source link

Add a `check.sh` parameter to allow the Docker lints to fail on errors. #35

Open TimD1 opened 2 weeks ago

TimD1 commented 2 weeks ago

Description It would be great if the check.sh script took an additional boolean parameter: FAIL_ON_ERRORS.

Use Case Most linters will return exit code 0 if no errors are found, and exit code 1 if any linting errors are found (mypy, ruff, npm-groovy-lint, flake8...). The current Docker image returns exit code 0 no matter what. If I want to run the provided Docker image with the desired behavior, I need to override the default entrypoint (providing lots of arguments):

docker run --entrypoint "/bin/bash" --mount type=bind,src=$PWD,dst=/data
  public.ecr.aws/aws-genomics/linter-rules-for-nextflow:v0.1 
-c "java  -Dorg.slf4j.simpleLogger.defaultLogLevel=error \
  -classpath linter-rules.jar:CodeNarc-3.3.0-all.jar:slf4j-api-1.7.36.jar:slf4j-simple-1.7.36.jar \
  org.codenarc.CodeNarc \
  -report=text:stdout \
  -rulesetfiles=rulesets/general.xml \
  -basedir=/data/ \
  -includes='**/*.nf' \
  -failOnError=true \
  -maxPriority3Violations=0 \
  -maxPriority2Violations=0 \
  -maxPriority1Violations=0"

Proposed Solution If check.sh had a boolean input FAIL_ON_ERRORS that behaves similarly to RULESET, this common behavior would be much easier to obtain:

docker run --mount type=bind,src=$PWD,dst=/data
  -e RULESET="general"
  -e FAIL_ON_ERRORS="true"
  public.ecr.aws/aws-genomics/linter-rules-for-nextflow:v0.1 

FAIL_ON_ERRORS would just add the additional flags:

  -failOnError=true \
  -maxPriority3Violations=0 \
  -maxPriority2Violations=0 \
  -maxPriority1Violations=0"
markjschreiber commented 1 week ago

This seems like a good idea. It might also be useful to have options like FAIL_ON_PRIORITY_LOW, FAIL_ON_PRIORITY_MED etc so that you can set your level of stringency to Priority >3, >2, or >1 violations.

Feel free to make a PR as I think it will be a few days before I can get to this.