mattiasgeniar / puppet-pre-commit-hook

A Puppet pre-commit hook to validate syntax

MIT License

60 stars 36 forks source link

Should the commit hook be opinionated about puppet-lint checks? #22

Closed rnelson0 closed 5 years ago

rnelson0 commented 5 years ago

The hook specifies some puppet-lint settings. This seems pretty opinionated and is better controlled by an individual's ~/.puppet-lint.rc or a module/controlrepo's ./.puppet-lint.rc. It also ties this hook to specific check names that could change (calling an invalid check name causes the program to exit). I suggest we remove those lines, short of any overriding reason they persist.

rnelson0 commented 5 years ago

23 would make this change.

Pros:

Future proofed against check name changes
Being less opinionated allows puppet-lint defaults (which reflect the style guide) to prevail

Cons:

May allow a user's individual preferences in ~/.puppet-lint.rc take preference if there is no config file in the repo, causing surprise when someone else runs tests, or when that user modifies their personal preferences.

I do feel the pros outweigh the cons, but maybe not?

mattiasgeniar commented 5 years ago

I completely agree, those puppet-lint defaults were made haphazardly and a long time ago, they are either no longer relevant, obsolete or in need of better defaults.

Best to serve this from a custom ~/.puppet-lint.rc.