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

Linting rules and examples of bad/good code with lint rules #2

Merged
merged 2 commits into from May 18, 2017

Conversation

kr8n3r
Copy link

@kr8n3r kr8n3r commented May 17, 2017

Spelling out some linting rules and setup.


## Linting rules

We use the following rules when linting files:
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a link to the config where these are defined?

Copy link
Author

Choose a reason for hiding this comment

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

not yet, will add one. it's very close to govuk_lint

- allow-single-line: true
class-name-format:
- 1
- convention: hyphenatedlowercase
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be hyphenatedbem?

no-debug: 1
no-duplicate-properties: 1
no-empty-rulesets: 1
no-extends: 0
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be good to prevent use of extend, can we set this to 1 instead so it'll warn?

no-important: 0
no-invalid-hex: 1
no-mergeable-selectors: 0
no-misspelled-properties: 0
Copy link
Contributor

Choose a reason for hiding this comment

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

no-misspelled-properties will enforce the correct spelling of CSS properties, this is probably sensible to check for too.

- convention: hyphenatedlowercase
property-sort-order: 0
property-units: 0
quotes: 0
Copy link
Contributor

Choose a reason for hiding this comment

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

It's probably worth picking one, for consistency. Double quotes?

  quotes:
    - 1
    - style: double

@kr8n3r
Copy link
Author

kr8n3r commented May 18, 2017

will go through all and also add good/bad examples

Copy link
Contributor

@gemmaleigh gemmaleigh left a comment

Choose a reason for hiding this comment

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

This looks good, two tiny things - I think we should link to the config/.sass-lint.yml file from the docs.

How is style.scss being used? Can we remove it from this commit if it isn't being referenced.

If the list of rules we are supporting could be headings, rather than bullet points - I think it would be easier to see where each example ends.

@kr8n3r
Copy link
Author

kr8n3r commented May 18, 2017

ups regarding styl.scss :(
linked to yaml and created headings from the list

@kr8n3r
Copy link
Author

kr8n3r commented May 18, 2017

rake aborted!
No Rakefile found (looking for: rakefile, Rakefile, rakefile.rb, Rakefile.rb)

@fofr
Copy link
Contributor

fofr commented May 18, 2017

Should this be using govuk-lint gem?
(And probably relevant: alphagov/govuk-lint#70)

@kr8n3r
Copy link
Author

kr8n3r commented May 18, 2017

don't think we need ruby hence sass-lint (not scss-lint ruby package), as it's node based

@fofr
Copy link
Contributor

fofr commented May 18, 2017

@igloosi First line reads "The following scss-lint", which led me to believe that's what's being used.

@kr8n3r
Copy link
Author

kr8n3r commented May 18, 2017

oh, the comments in the sass-lint file
to avoid confusion I removed that line that references scss-linter rules

Explain linting rules and provide bad/good code examples
@kr8n3r kr8n3r changed the title [WIP] linting rules wording Linting rules and examples of bad/good code with lint rules May 18, 2017
# Linting

To ensure code quality and consistency in our Sass files we check that certain style rules are followed using a project [YAML file](../config/.sass-lint.yml)
As we're using node-sass parser to parse our scss files, the packing of choice is [sass-lint](https://github.com/sasstools/sass-lint).
Copy link
Contributor

Choose a reason for hiding this comment

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

package?

Copy link
Author

Choose a reason for hiding this comment

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

yeah...

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.

None yet

4 participants