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

Fix lint #104

Merged
merged 3 commits into from
Jan 29, 2021
Merged

Fix lint #104

merged 3 commits into from
Jan 29, 2021

Conversation

schurzi
Copy link
Contributor

@schurzi schurzi commented Jan 26, 2021

No description provided.

Signed-off-by: Martin Schurz <Martin.Schurz@t-systems.com>
Signed-off-by: Martin Schurz <Martin.Schurz@t-systems.com>
@schurzi schurzi mentioned this pull request Jan 26, 2021
@deric4
Copy link
Member

deric4 commented Jan 26, 2021

From technical standpoint, everything looks good. I do have a question though, are Inspec Profiles expected to adhere to the default cops or should there be an attempt to align with the Inspec Docs/Style Guide:

Avoiding unnecessary parentheses in matchers:
https://docs.chef.io/inspec/style/#avoid-unnecessary-parentheses-in-matchers

some examples here: #90 (comment)

@schurzi
Copy link
Contributor Author

schurzi commented Jan 28, 2021

nice catch @deric4.

I already disabled Lint/AmbiguousBlockAssociation because of this. In the case here I would also need to disable Lint/AmbiguousRegexpLiteral. when this is done the autofix will no longer meddle with these parentheses.

I think the best approach is to get this in a state, where we can trust the autofix and end up with consistent code. The best(tm) approach should be to trust the linter completely, since every disabled rule leads to possible ambiguity and we need people to remember the special inspec rules.

@chris-rock what do you think?

@chris-rock
Copy link
Member

I agree that it is confusing to have two different baselines. I would argue that Inspec policies are more geared towards security professionals, therefore it is okay to deviate. In any case we should make sure its the same for cross-policies for our baselines.

Personally, I would go with the simpler solution in this case, therefore I prefer it { should eq value }.

Signed-off-by: Martin Schurz <Martin.Schurz@t-systems.com>
@schurzi
Copy link
Contributor Author

schurzi commented Jan 28, 2021

thank you for clearing this up. The code is now aligned with that decision.

Copy link
Member

@chris-rock chris-rock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very nice clean up @schurzi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants