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

[Do not merge] Enable sass linting in Jenkins #402

Closed
wants to merge 3 commits into from
Closed

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Jul 19, 2017

Jenkinsfile docs suggest that sass linting is on by default:

“but aren't linting your SASS yet (you should), you can disable linting”
“sassLint Whether or not to run the SASS linter. Default: true”

But the logic says:
if (hasAssets() && hasLint() && options.sassLint)

If the option is omitted, it will default to false.

Before this PR, the Sass linter does not run:
https://ci.integration.publishing.service.gov.uk/job/government-frontend/job/display-curated-links-in-sidebar/5/console

You can see it does run now:
https://ci.integration.publishing.service.gov.uk/job/government-frontend/job/enable-sass-linting/1/console

fofr added 2 commits Jul 19, 2017
Jenkinsfile docs suggest that sass is on by default:
“but aren't linting your SASS yet (you should), you can disable linting”
“sassLint Whether or not to run the SASS linter. Default: true”

But the logic says:
`if (hasAssets() && hasLint() && options.sassLint)`

If the option is omitted, it will default to false.
@boffbowsh boffbowsh temporarily deployed to government-frontend-pr-402 Jul 19, 2017 Inactive
@fofr fofr changed the title [Do not merge] Enable sass linting [Do not merge] Enable sass linting in Jenkins Jul 19, 2017
@tijmenb
Copy link
Contributor

@tijmenb tijmenb commented Jul 19, 2017

Hmm, this seems like a bug in the puppet code, we intended to make it hard to not run the sass-linter.

I think the default value was inadvertently removed in alphagov/govuk-puppet@43af3ed. We should fix it there.

@fofr
Copy link
Contributor Author

@fofr fofr commented Jul 19, 2017

Check sass linter still runs in Jenkins
@fofr
Copy link
Contributor Author

@fofr fofr commented Jul 21, 2017

Closing. Puppet change has fixed this: alphagov/govuk-puppet#6131

Reverting the change in a commit, CI runs the sass linter and fails the build as expected:
https://ci.integration.publishing.service.gov.uk/job/government-frontend/job/enable-sass-linting/3/console

bundle exec govuk-lint-sass app/assets/stylesheets
app/assets/stylesheets/helpers/_available-languages.scss:1:1 [W] IdSelector: Avoid using id selectors
app/assets/stylesheets/helpers/_available-languages.scss:2:10 [W] ColorVariable: Color literals like `#000` should only be used in variable declarations; they should be referred to via variable everywhere else.
@fofr fofr closed this Jul 21, 2017
@fofr fofr deleted the enable-sass-linting branch Jul 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.