Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

wip fix(deps): update dev dependencies #2883

Merged
merged 1 commit into from
Aug 10, 2015
Merged

Conversation

vladikoff
Copy link
Contributor

No description provided.

@vladikoff
Copy link
Contributor Author

@pdehaan this will fail for a few reasons but one of them is eslint's indent,

app/scripts/models/account.js
  119:10  error  Expected indentation of 6 characters but found 8  indent

app/scripts/models/auth_brokers/first-run.js
  23:7  error  Expected indentation of 4 characters but found 5  indent

app/scripts/models/user.js
  222:16  error  Expected indentation of 12 characters but found 14  indent

Do you know why is that? the indent it suggests is pretty ugly...

@vladikoff vladikoff changed the title fix(deps): update dev dependencies wip fix(deps): update dev dependencies Aug 5, 2015
@pdehaan
Copy link
Contributor

pdehaan commented Aug 5, 2015

@vladikoff Works for me against current master, so I'll go ahead and blame some change in eslint@1.

More accurately, I agree that errors 1 and 3 are ugly indenting. Error 2 above seems like our bad.

Let me grab your branch and try rerunning this against the newer deps. Not sure why ESLint would only flag those two lines though. Maybe because results 1 and 3 both follow empty functions (only have a comment) and it's getting confused. I'd hate to add an // eslint-disable-line indent on those two lines, but i'm pretty sure that ESLint will release a 1.0.1 by end of week that will fix that. Testing continues.

@pdehaan
Copy link
Contributor

pdehaan commented Aug 5, 2015

@vladikoff Yeah, I can repro on your branch locally.

It seems that eslint isn't liking that empty function's indenting. Doesn't seem to complain if I put a void 0; or return; in there, but that's kind of gross. I'll try boiling this down into a simple test case and file upstream w/ eslint and see what they say. Definitely looks like a change between eslint@0.24.1 and eslint@1.0.0.

    _fetchProfileOAuthToken: function () {
      var self = this;
      return self.createOAuthToken(PROFILE_SCOPE)
        .then(function (accessToken) {
          self.set('accessToken', accessToken.get('token'));
        }, function () {
          // Ignore errors; we'll just fetch again when needed
          return;
        });
    },

UPDATE: Filed upstream as eslint/eslint#3279

@vladikoff vladikoff removed the WIP label Aug 10, 2015
vladikoff added a commit that referenced this pull request Aug 10, 2015
fix(deps): update dev dependencies
@vladikoff vladikoff merged commit c25ec8a into mozilla:master Aug 10, 2015
@vladikoff vladikoff deleted the devdeps44 branch August 10, 2015 14:25
@pdehaan pdehaan mentioned this pull request Aug 10, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants