Code Monkey home page Code Monkey logo

Comments (13)

asottile avatar asottile commented on August 19, 2024 4

Nearly every linter supports as a standard interface linter [filename, [filename ...]] (except this one) please consider the precedents set by tools like pyflakes / flake8 / pylint / eslint / prettier / black / etc.

Not only does this make invocation dead simple with xargs, it is much more performant (the interpreter and subprocess overhead occurs once instead of a linear amount of times by file). This overhead is especially high on windows. (even on Linux, compare the time taken between git ls-files -- '*.py' | xargs pylint vs. git ls-files -- '*.py' | xargs -n1 pylint).

Also in line with other posix tools you could simply invoke with a glob linter *.yaml instead of needing to copy paste a complicated wrapper everywhere.

The main motivation here is so that this linter can easily be integrated into other frameworks, in particular https://pre-commit.com

from cfn-lint.

asottile avatar asottile commented on August 19, 2024

I started poking at this in #152

Here's a patch that gets closer (on top of that branch):

diff --git a/src/cfnlint/core.py b/src/cfnlint/core.py
index df383ac..2b10743 100644
--- a/src/cfnlint/core.py
+++ b/src/cfnlint/core.py
@@ -93,9 +93,11 @@ def create_parser():
 
     # Alllow the template to be passes as an optional or a positional argument
     parser.add_argument(
-        'template', nargs='?', help='The CloudFormation template to be linted')
+        'templates', nargs='*', default=[], help='The CloudFormation template to be linted')
     parser.add_argument(
-        '-t', '--template', dest='template_alt', help='The CloudFormation template to be linted')
+        '-t', '--template', dest='templates', action='append',
+        help='The CloudFormation template to be linted'
+    )
 
     parser.add_argument(
         '-b', '--ignore-bad-template', help='Ignore failures with Bad template',
@@ -185,14 +187,7 @@ def get_template_args_rules(cli_args):
     fmt = args.format
     formatter = get_formatter(fmt)
 
-    # Filename can be speficied as positional or optional argument. Positional
-    # is leading
-    if args.template:
-        filename = args.template
-    elif args.template_alt:
-        filename = args.template_alt
-    else:
-        filename = None
+    filenames = args.templates
 
     if filename:
         ignore_bad_template = args.ignore_bad_template

from cfn-lint.

fatbasstard avatar fatbasstard commented on August 19, 2024

Personally I'm nog a big fan of handling multiple files in the linter...

The linter should focus on linting a template. Adding a wrapper layer to work/process multiple files only add complexity and possible errors. Apart from the fact that you don’t know how you want the output then.

If you want to check multiple templates, that’s really no issue to build that outside of the linter.
Create a bash script that loops over a folder of parses multiple files and call the linter.
Or implement the Python module in your own codebase and handle the multiple file logic there.

from cfn-lint.

cmmeyer avatar cmmeyer commented on August 19, 2024

I'm with @fatbasstard on this one -- I worry the output will get complex for multiple files and move into land they may make IDE integration more complex. But not reason you couldn't write a "batch" wrapper script using the core libraries just like we do for the IDE wrappers.

from cfn-lint.

alexjurkiewicz avatar alexjurkiewicz commented on August 19, 2024

Yep, when I'm linting my CFN files I can run yaml-lint $files but need a special case for file in $files ; do cfn-lint $file which is annoying.

from cfn-lint.

asottile avatar asottile commented on August 19, 2024

(just to put numbers to what I tapped out on my phone above re: pylint):

$ uname -a
Darwin Macbook-Pro 17.6.0 Darwin Kernel Version 17.6.0: Tue May  8 15:22:16 PDT 2018; root:xnu-4570.61.1~1/RELEASE_X86_64 x86_64 i386 MacBookPro11,4 Darwin
$ git ls-files -- '*.py' | grep ^src/cfnlint | time xargs .tox/pylint36/bin/pylint 
...
       11.92 real        11.59 user         0.31 sys
$ git ls-files -- '*.py' | grep ^src/cfnlint | time xargs -n1 .tox/pylint36/bin/pylint
...
       75.78 real        69.26 user         5.93 sys

from cfn-lint.

alexjurkiewicz avatar alexjurkiewicz commented on August 19, 2024

As per #166 and #172, a proposal for how to handle migration from single-template to multi-template support:

"Next" version:

  1. Add --templates, which accepts multiple template files (comma-separated) and can be specified multiple times (eg --templates test1.yml,test2.json --templates test3.yaml)
  2. Change -t to mean --templates
  3. Change template position argument to allow zero or more (*)
  4. Deprecate --template (suppress display in usage, and print warning to stderr if it's used)
  5. When only one file is specified, output format is unchanged from not (except the warning is printed if applicable)
  6. When multiple files are specified, output format can change if required

"Next+1" version:

  1. Remove --template

from cfn-lint.

kddejong avatar kddejong commented on August 19, 2024

@alexjurkiewicz We had talked about (-r, -i, -a) being switched to append (argparse) or comma delimited list. I don't see that above is that still part of this?

from cfn-lint.

alexjurkiewicz avatar alexjurkiewicz commented on August 19, 2024

I think that can be done as a separate PR, so I'll do that. Are you worried about backwards compatibility for those options with multiple items?

from cfn-lint.

hoshsadiq avatar hoshsadiq commented on August 19, 2024

This would be cool, as this could potentially allow validating outputs when using nested stacks.

from cfn-lint.

adamchainz avatar adamchainz commented on August 19, 2024

I'm +1 on multiple files being supported, it would make linting much faster. Currently a new Python process has to be spawned and all the code imported for each, a great overhead. Most linters I know of support multiple files.

from cfn-lint.

cmmeyer avatar cmmeyer commented on August 19, 2024

Just merged #331 which implements this enhancement!

from cfn-lint.

kddejong avatar kddejong commented on August 19, 2024

Released in v0.7.4

from cfn-lint.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.