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

eslint: upgrade to 1.10.3 #1807

Merged
merged 12 commits into from
Dec 18, 2015
Merged

eslint: upgrade to 1.10.3 #1807

merged 12 commits into from
Dec 18, 2015

Conversation

blowery
Copy link
Contributor

@blowery blowery commented Dec 18, 2015

Move to 1.10.3 and 3.11.3 of the react plugin

To test, run make lint and it doesn't break

@blowery blowery added Framework [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. Build labels Dec 18, 2015
@blowery blowery self-assigned this Dec 18, 2015
@blowery blowery changed the title eslint: upgade to current latest eslint: upgade to 1.10.3 Dec 18, 2015
@blowery blowery changed the title eslint: upgade to 1.10.3 eslint: upgrade to 1.10.3 Dec 18, 2015
@blowery blowery force-pushed the update/eslint branch 2 times, most recently from a3bc840 to 43f5d3b Compare December 18, 2015 15:02
@sirbrillig
Copy link
Member

Works for me! Should we fix the lint-reported errors in this PR also?

eg:

diff --git a/client/lib/preferences/test/actions.js b/client/lib/preferences/test/actions.js
index 730ffc4..55ea30a 100644
--- a/client/lib/preferences/test/actions.js
+++ b/client/lib/preferences/test/actions.js
@@ -98,7 +98,7 @@ describe( 'PreferencesActions', function() {
            } );
        } );

-       it( 'should not dispatch an empty local store', function() {
+       it( 'should not dispatch an empty local store', function( done ) {
            store.get.restore();
            sandbox.stub( store, 'get' ).returns( undefined );

@blowery blowery added [Status] In Progress and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 18, 2015
@blowery
Copy link
Contributor Author

blowery commented Dec 18, 2015

Should we fix the lint-reported errors in this PR also?

Yes, have to, as they break the build. I was just pushing this up to track the breakage. Should have marked it In Progress, but ... yeah.

@sirbrillig
Copy link
Member

I fixed a pile of them. The few remaining I'm not sure about (not sure how to use notices correctly since the refactor and I'm guessing maybe tinymce is global?).

@sirbrillig
Copy link
Member

ping @codebykat for the tinymce errors:

/home/ubuntu/wp-calypso/client/components/tinymce/plugins/wpcom-charmap/charmap.jsx
  309:61  error  "tinymce" is not defined  no-undef

/home/ubuntu/wp-calypso/client/components/tinymce/plugins/wpcom-charmap/plugin.js
  49:2  error  "tinymce" is not defined  no-undef

@ehg
Copy link
Member

ehg commented Dec 18, 2015

tinymce is global

Yep. It's attached to the window object when the editor is loaded/pre-loaded.

@sirbrillig
Copy link
Member

Ping @deBhal for the translator notices reference:

/home/ubuntu/wp-calypso/client/layout/community-translator/invitation-utils.js
  166:5  error  "notices" is not defined  no-undef

@nylen
Copy link
Contributor

nylen commented Dec 18, 2015

Fixed up the editor charmap in c415e0f. TinyMCE is a global but we shouldn't use it like that unless absolutely necessary.

@blowery blowery added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Dec 18, 2015
@nylen
Copy link
Contributor

nylen commented Dec 18, 2015

The notices fix looked simple enough too (require( 'notices' ).error is used in lots of places).

I think this is good to go 👍

@sirbrillig
Copy link
Member

:shipit:

blowery added a commit that referenced this pull request Dec 18, 2015
@blowery blowery merged commit 1cd903d into master Dec 18, 2015
@scruffian scruffian removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Dec 18, 2015
@sirbrillig sirbrillig deleted the update/eslint branch December 18, 2015 19:48
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.

5 participants