Lint codebase using standard #334

NickColley · 2016-09-29T10:34:58Z

I recognise this is a big diff, so just raising this to see what people think..

Best viewed commit by commit, any changes that could potentially change logic or were manually I've done in different commits - separate from the automated fixing.

Linting the codebase has removed unused variables, made us explicit about declaring globals and improved patterns like the following

Foo(function (options) {
  var options = options || {}
}))

Foo(function (options) {
  options = options || {}
}))

Finally I've silenced an error surrounding our use of 'new', this indicates an issue with our module pattern wherein we don't need to be using a class pattern for some of our code since there's no returned instance that is being used.

If this is the case that there are public methods exposed on an instance we should test those...

tvararu

Great work! 👍

I didn't review the --fix commit as it's massive and I trust the AST-parsing robots better than I trust myself.

tvararu · 2016-09-29T10:45:34Z

package.json

    "grunt-contrib-jasmine": "^1.0.0",
    "grunt-contrib-sass": "0.7.4",
-    "jquery": "~1.11.3"
+    "jquery": "~1.11.3",
+    "standard": "^8.2.0"


NB: Standard 8 was launched this month but govuk_prototype_kit for ex is still on Standard 7.

Updating is not really a priority IMO. (the only important changes are for JSX I think which we don't use)

tvararu · 2016-10-11T12:39:33Z

Can you add semicolons at the start of IIFE's as part of this? Will solve #338.

This indicates an issue with our module pattern wherein we don't need to be using a class pattern for some of our code since there's no returned instance that is being used. If this is the case that there are public methods exposed on an instance we should test those...

- Lint codebase using standard ([PR #334](#334)) - Add semicolons at the start of IIFE's ([PR #339](#339))

- Removed phase-banner scss file as no longer needed Full list of changes: SelectionButtons will add a class to the label with the type of the child input alphagov/govuk_frontend_toolkit#317 Add GOVUK.ShowHideContent JavaScript to support showing and hiding content, toggled by radio buttons and checkboxes alphagov/govuk_frontend_toolkit#315 Fix error in IE - remove trailing comma from shimLinksWithButtonRole JavaScript alphagov/govuk_frontend_toolkit#323 Remove unnecessary print font fallback that causes regression downstream alphagov/govuk_frontend_toolkit#328 Update tooling to use npm script rather than grunt-shell alphagov/govuk_frontend_toolkit#327 For smaller screens (<768px) ensure that the GOVUK.StickAtTopWhenScrolling JS "unsticks" the element which was previously "stuck" (by removing both the class which sets fixed positioning and the shim). alphagov/govuk_frontend_toolkit#329 Lint codebase using standard alphagov/govuk_frontend_toolkit#334 Add semicolons at the start of IIFE's alphagov/govuk_frontend_toolkit#339 Removal of external link styles and icons, if you are using the external-link-* mixins you will need to remove them from your codebase alphagov/govuk_frontend_toolkit#293 Correct spelling of the 'accordion' icon, you will need to check for the incorrect spelling 'accordian' and update if you are using this icon alphagov/govuk_frontend_toolkit#345 Amend GOVUK.StickAtTopWhenScrolling to resize the sticky element and shim when the .js-sticky-resize class is set alphagov/govuk_frontend_toolkit#343 Allow custom options in GOVUK.analytics.trackPageview alphagov/govuk_frontend_toolkit#332 Fix role="button" click shim alphagov/govuk_frontend_toolkit#347 Make font variables lowercase alphagov/govuk_frontend_toolkit#348 Update selection button documentation alphagov/govuk_frontend_toolkit#350 Change colour used in phase tags to govuk-blue alphagov/govuk_frontend_toolkit#353

* Test track-share-button-clicks module * Lint using JS Standard (as per alphagov/govuk_frontend_toolkit#334) * Include jQuery as a spec helper

The toolkit JS was refactored to pass StandardJS linting rules: alphagov/govuk_frontend_toolkit#334 We haven't moved to them yet and still lint according to JSHint + our own config (see .jshintrc). This refactors the toolkit JS to pass our rules. Note: we will move to StandardJS rules in the future but doing so requires changing all our JS and tests so cannot be included in this story.

NickColley force-pushed the standard branch from 69f9d14 to b6ec37a Compare September 29, 2016 10:35

tvararu approved these changes Sep 29, 2016

View reviewed changes

NickColley mentioned this pull request Oct 3, 2016

Amend GOVUK.stickAtTopWhenScrolling to "unstick" for smaller screens #329

Merged

NickColley added 7 commits October 19, 2016 15:30

Add standard as linter to npm test

3b81dd2

Run standard --fix to fix most problems automatically

c66c4cb

Fix manual linting issues without logic changes

1bf2ad6

Use strict equals to pass linting

0cdc894

Ignore _SpecRunner.html generated file from testing

023a350

Add linting note to README

b32fa4c

robinwhittleton force-pushed the standard branch from b6ec37a to b32fa4c Compare October 19, 2016 14:34

robinwhittleton mentioned this pull request Oct 19, 2016

Mixing of semi-colon styles can cause errors #338

Closed

robinwhittleton merged commit ca84786 into master Oct 19, 2016

robinwhittleton deleted the standard branch October 19, 2016 14:47

robinwhittleton changed the title ~~[Do note merge] [Request for comments] Lint codebase using standard~~ Lint codebase using standard Oct 19, 2016

gemmaleigh added a commit that referenced this pull request Oct 20, 2016

v 4.18.4

4a2828b

- Lint codebase using standard ([PR #334](#334)) - Add semicolons at the start of IIFE's ([PR #339](#339))

gemmaleigh mentioned this pull request Oct 20, 2016

v4.18.4 #341

Merged

gemmaleigh added a commit that referenced this pull request Oct 20, 2016

v 4.18.4

0991e23

- Lint codebase using standard ([PR #334](#334)) - Add semicolons at the start of IIFE's ([PR #339](#339))

Guntrisoft mentioned this pull request Dec 2, 2016

Upgrade frontend toolkit to version 5.0.2 guidance-guarantee-programme/pension_guidance#700

Merged

tvararu mentioned this pull request Dec 16, 2016

Update JavaScript styleguide to use Standard alphagov/styleguides#97

Closed

fofr mentioned this pull request Dec 16, 2016

Add "Standard" javascript linting alphagov/govuk-lint#63

Closed

fofr mentioned this pull request May 18, 2017

Upgrade NodeJS alphagov/govuk-puppet#5942

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lint codebase using standard #334

Lint codebase using standard #334

NickColley commented Sep 29, 2016 •

edited

Loading

tvararu left a comment

tvararu Sep 29, 2016 •

edited

Loading

tvararu commented Oct 11, 2016

Lint codebase using standard #334

Lint codebase using standard #334

Conversation

NickColley commented Sep 29, 2016 • edited Loading

tvararu left a comment

Choose a reason for hiding this comment

tvararu Sep 29, 2016 • edited Loading

Choose a reason for hiding this comment

tvararu commented Oct 11, 2016

NickColley commented Sep 29, 2016 •

edited

Loading

tvararu Sep 29, 2016 •

edited

Loading