Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Puppethack: Validate puppet code before attempting to lint it. #586

Open
wants to merge 4 commits into
base: 3.0.0
Choose a base branch
from

Conversation

rnelson0
Copy link
Collaborator

puppet-lint is a linter and not a validator. Invalid code causes many lexing errors.

@rnelson0 rnelson0 changed the title Validate puppet code before attempting to lint it. Puppethack: Validate puppet code before attempting to lint it. Dec 13, 2016
@rnelson0 rnelson0 mentioned this pull request Dec 13, 2016
3 tasks
@rnelson0 rnelson0 added this to the 3.0.0 milestone Dec 17, 2016
@binford2k
Copy link
Collaborator

binford2k commented Dec 21, 2016

I really really like this. And I would love to be able to use Puppet's lexer directly in the future for certain checks. On the other hand, requiring the puppet gem is setting major precedent here. Is it a big deal that we're now pulling in a bunch more dependencies?

I'm 👍 👍 👍 on this.

@rnelson0
Copy link
Collaborator Author

rnelson0 commented Dec 21, 2016 via email

@binford2k
Copy link
Collaborator

Puppet.settings[:app_management] = true if Gem::Version.new(Puppet.version) >= Gem::Version.new('4.3.2')

@rnelson0
Copy link
Collaborator Author

In yesterday's triage, we discussed the need to support AppOrchestration code in the API, with the code that @binford2k provided. I've added this to the PR.

We also discussed the desire to continue processing additional files on the command line when validation of a file fails, but that underlying issue is due to the loop construct bailing early without a rescue inside it (here) which should be tracked separately in #603.

@rnelson0
Copy link
Collaborator Author

This is good to review as Ruby 1.8.7 will be out of the matrix soon enough. @binford2k @rodjek

@glennsarti
Copy link

Would it be possible to add an option to skip the parser validation? I'm currently consuming the puppet-lint gem and I already have checked for a welformed manifest prior to linting. It may also get around the issue of requiring the puppet gem, whereas it could be optional if it exists.

@rnelson0
Copy link
Collaborator Author

I think those would both be possible to do, though the puppet gem dependency could be tricky to navigate.

@rodjek
Copy link
Owner

rodjek commented Jun 17, 2017

Yeah, I agree that this should be an optional thing. Wrap the require with begin ... rescue LoadError and then short circuit valid? if Puppet isn't defined.

@glennsarti
Copy link

Doing the begin...rescue will be fine for Puppet detection but doesn't make it a configurable option. Would this be a lint check that you could disable or a cmd line option to puppet-lint?

@david22swan
Copy link

@rnelson0 Hey, its been a while since this has been updated and I was wondering whether you intended any further work on it?

@rnelson0
Copy link
Collaborator Author

@david22swan Not me, personally, I unfortunately do not have the time to participate in this project nearly as much these days. I believe there may have been some improvements that supersede its need anyway, perhaps @rodjek can confirm if that is the case, or if there is already something in the plans.

@DavidS
Copy link
Collaborator

DavidS commented Nov 9, 2020

There's #728 which adds some error detection and handling. If this PR is to be merged, it'll need to be rebased and de-conflicted with that change first.

@DavidS DavidS self-assigned this Nov 9, 2020
@DavidS DavidS added the feature label Nov 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants