Skip to content

Conversation

@aaxelb
Copy link
Contributor

@aaxelb aaxelb commented Mar 28, 2018

Purpose

Fix all linter warnings and consider them unacceptable from now on.

Summary of Changes

  • add --max-warnings=0 to eslint invocations in .travis.yml and package.json, so warnings behave like errors on travis and pre-commit hooks
  • Fix all the warnings!
  • There were a few times I was feeling feisty, so I converted some things to typescript. But most of the time I wasn't, so I didn't.

Side Effects / Testing Notes

This touched lots of things but shouldn't have changed any. Warrants a regression test.

Ticket

https://openscience.atlassian.net/browse/EMB-172

Reviewer Checklist

  • meets requirements
  • easy to understand
  • DRY
  • testable and includes test(s)
  • changes described in CHANGELOG.md

@coveralls
Copy link

coveralls commented Mar 28, 2018

Coverage Status

Coverage increased (+0.04%) to 20.728% when pulling c85b836 on aaxelb:emb-172--no-warning into c945473 on CenterForOpenScience:develop.

},
});
authenticate() {
return this._test();
Copy link
Member

@jamescdavis jamescdavis Mar 28, 2018

Choose a reason for hiding this comment

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

So it doesn't actually have to be wrapped in an RSVP promise?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed authenticatedAJAX to take care of that, since it was the source of the jQuery promise. Now no one else has to worry.

@aaxelb aaxelb force-pushed the emb-172--no-warning branch from 9f3213c to e3d80e3 Compare March 29, 2018 17:39
@jamescdavis
Copy link
Member

Muchos conflictos. Lo siento. :(

Copy link
Member

@jamescdavis jamescdavis left a comment

Choose a reason for hiding this comment

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

💯

@jamescdavis
Copy link
Member

This is now mostly duplicated by #144. There may be some other useful stuff here that we might want to include in a separate PR.

@jamescdavis jamescdavis closed this Apr 3, 2018
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.

3 participants