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

Disable linters #677

Merged
merged 16 commits into from
Nov 6, 2016
Merged

Disable linters #677

merged 16 commits into from
Nov 6, 2016

Conversation

DanPurdy
Copy link
Member

@DanPurdy DanPurdy commented May 5, 2016

Ongoing work for disabling linters via source.

closes #70

Ongoing from the work of @donabrams in #402

Everyone feel free to contribute to this branch feature/disable-linters

@avinashci
Copy link

avinashci commented May 12, 2016

To start with the issue is in index.js file. In the sassLint.lintText method, following statements should be called after the ast is generated.
ruleToggles = getToggledRules(ast);
isEnabledFilter = isResultEnabled(ruleToggles);

The above lines should move inside the block that runs the rules.

if (ast.content && ast.content.length > 0) {
ruleToggles = getToggledRules(ast);
isEnabledFilter = isResultEnabled(ruleToggles);

I made the changes locally and the tests for rules are passing.

@DanPurdy
Copy link
Member Author

Thanks @avinashci as mentioned there has been a lot of changes to sass-lint since this was last worked on including the block of code you highlighted the result you're seeing is just the result of the work being brought up to date with the current version and is unfortunately not finished..

Thanks for that catch though, feel free to open a PR into the disable linter branch to contribute that change. We'll be moving forward with updating this all very soon

@DanPurdy
Copy link
Member Author

Please do not add comments in PR's unless they add something to the discussion. Issues can be used to request features and github's reactions can be used to show your support for things.

@saptar
Copy link

saptar commented May 19, 2016

submitted a pull request #723 to incorporate changes for programatically enabling/disabling linting ; as a part of issue # 70.

@saptar
Copy link

saptar commented May 23, 2016

Hi @DanPurdy ,
I have updated the PR #723 .
Thanks!
-Saptarshi

@coveralls
Copy link

coveralls commented May 23, 2016

Coverage Status

Coverage decreased (-0.03%) to 97.237% when pulling c9a3b65 on feature/disable-linters into b2f8523 on develop.

@avinashci
Copy link

Looks like the merge issues are resolved and the changes have been integrated to the branch. Anymore help needed here?

@DanPurdy
Copy link
Member Author

DanPurdy commented Jun 3, 2016

There are no tests for .sass syntax and I've not had time to go through it properly but I will.. Yeah the main thing holding it back at the moment is the sass tests

@saptar
Copy link

saptar commented Jun 17, 2016

Hi @DanPurdy ,

I have created a PR #743 to add test cases for .sass syntax.

Thanks!

@goesbysteve
Copy link

My team mates and I would like to contribute to this. How best can we help? I realise we are a little late to the party;-)

@DanPurdy
Copy link
Member Author

DanPurdy commented Jul 5, 2016

Hi @stevegibbings, Thanks for the offer any help provided would be appreciated (personally struggling to find time at moment!)

There's an issue in #743 that would need looking at in order to make sure the Sass syntax is working correctly. It looks like an issue with Sass and CRLF line endings.

Alternatively throwing a few more test cases at both syntaxes with a view to just releasing this functionality for scss in the meantime would be appreciated.

Basically any help or light that can be shed on this feature would be greatly appreciated.

@coveralls
Copy link

coveralls commented Jul 7, 2016

Coverage Status

Coverage decreased (-0.04%) to 97.337% when pulling aaa452c on feature/disable-linters into 6f5a509 on develop.

cappadona added a commit to cul-it/mann-wagon that referenced this pull request Jul 15, 2016
Wholesale consistency via sass-lint. Style for Vue components will be addressed
in the next commit.

There are a couple of outliers related to constraints inherent in vendored tools
(semantic-ui & jquery plugins). These will be resolved once the ability to
disable linters from the source is implemented. sasstools/sass-lint#677
@coveralls
Copy link

coveralls commented Aug 8, 2016

Coverage Status

Coverage decreased (-0.1%) to 97.242% when pulling 535ba4d on feature/disable-linters into 6f5a509 on develop.

@DanPurdy
Copy link
Member Author

Ah that's a bug with a previous version of our AST, shouldn't cause any issues now when we release the latest version of Sass-lint

@coveralls
Copy link

coveralls commented Oct 28, 2016

Coverage Status

Coverage decreased (-0.04%) to 97.536% when pulling 000f3a0 on feature/disable-linters into 21a20eb on develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 97.54% when pulling b7a8832 on feature/disable-linters into d67549d on develop.

1 similar comment
@coveralls
Copy link

coveralls commented Nov 2, 2016

Coverage Status

Coverage decreased (-0.04%) to 97.54% when pulling b7a8832 on feature/disable-linters into d67549d on develop.

@coveralls
Copy link

coveralls commented Nov 2, 2016

Coverage Status

Coverage decreased (-0.04%) to 97.54% when pulling 96bd89d on feature/disable-linters into d67549d on develop.

@coveralls
Copy link

coveralls commented Nov 2, 2016

Coverage Status

Coverage decreased (-0.04%) to 97.54% when pulling d26b74b on feature/disable-linters into d67549d on develop.

@DanPurdy
Copy link
Member Author

DanPurdy commented Nov 6, 2016

Will release with 1.10

@DanPurdy DanPurdy merged commit 4cd8a98 into develop Nov 6, 2016
@albell
Copy link

albell commented Nov 6, 2016

@DanPurdy Thank you! When should folks generally expect 1.10 to roll out?

@bgriffith
Copy link
Member

@albell It's already out! 🎉

@DanPurdy DanPurdy deleted the feature/disable-linters branch November 19, 2016 17:21
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.

Feature Request: Enable Disabling Linters via Source