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

Don't ignore comments in no-empty-rulesets #998

Merged
merged 2 commits into from
Aug 27, 2017

Conversation

royaldark
Copy link
Contributor

This commit makes the no-empty-rulesets rule trigger when a ruleset
contains only comments.

Closes #968.

DCO 1.1 Signed-off-by: Joe Einertson <jeinerts@umm>

This commit makes the no-empty-rulesets rule trigger when a ruleset
contains only comments. Closes sasstools#968.
@coveralls
Copy link

coveralls commented Jan 17, 2017

Coverage Status

Coverage remained the same at 97.518% when pulling 7d94ef0 on royaldark:develop into d96ed96 on sasstools:develop.

Copy link
Member

@DanPurdy DanPurdy left a comment

Choose a reason for hiding this comment

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

This was actually a decision taken at the beginning of the rule as far as i can remember. I'll try finding the original issue, but it was discussed that this check should be behind a flag as some people wanted to keep certain rulesets empty. (for a reason i can't remember).

@royaldark
Copy link
Contributor Author

I thought from the discussion in #968 it would be okay since rules can now be disabled via comments. Putting it in a flag isn't much work, but I guess I don't see the use case for allowing empty blocks at all. I tried searching through issues to find relevant discussions but didn't find much.

@coveralls
Copy link

coveralls commented Aug 27, 2017

Coverage Status

Coverage remained the same at 97.511% when pulling 84796df on royaldark:develop into b5aebda on sasstools:develop.

@DanPurdy
Copy link
Member

Yeah, you're completely right here. Sorry it's taken so long to come back to this.

@DanPurdy DanPurdy merged commit 2c4f63e into sasstools:develop Aug 27, 2017
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.

3 participants