update ember-cli. update tests. use ES6. cleanup.

amkirwan / ember-oauth2

JavaScript library for using OAuth 2.0 Implicit Grant flow (Client-Side Flow) with Ember.js

MIT License

133 stars 22 forks source link

update ember-cli. update tests. use ES6. cleanup. #37

Closed miguelcobain closed 5 years ago

miguelcobain commented 5 years ago

This PR:

updates to the latest ember-cli version (this also removes the ember-cli-babel deprecation we're seeing on our apps)
adds a LICENSE.md file
uses let instead of var
uses ES6 function shorthand syntax
updates tests to use the new testing API and new Sinon version
uses warn instead of Ember.Logger
uses new modules API (no more Ember. global usage)

This PR supersedes #36.

It would be awesome to get a new release for this addon ASAP, @amkirwan. Thanks!

Meanwhile, people can use this branch by running:

yarn add -D github:miguelcobain/ember-oauth2#master

npm install --save-dev github:miguelcobain/ember-oauth2#master

miguelcobain commented 5 years ago

@amkirwan I think this is a great PR for the project. Any blockers?

amkirwan commented 5 years ago

@miguelcobain thanks for the PR will take a look in the next day or two

miguelcobain commented 5 years ago

@amkirwan any news?

patocallaghan commented 5 years ago

This is also something I'd love to see merged. ember-oauth2 is the only addon we have in our app that runs on ember-cli-babel@5.

miguelcobain commented 5 years ago

@amkirwan Monthly ping.

If you'd like me to help, you can add me as a maintainer.

We've been using our fork in production for some months now without issues.

patocallaghan commented 5 years ago

@miguelcobain I think we're going to opt to use your branch in prod too until this is merged. Thanks for putting it together 👍

patocallaghan commented 5 years ago

FYI @miguelcobain I've opened an issue to see whether this addon can be "put up for adoption" https://github.com/amkirwan/ember-oauth2/issues/38

miguelcobain commented 5 years ago

@amkirwan regarding your const suggestions, please read this article by mixonic that explains this whole let vs const debate: https://madhatted.com/2016/1/25/let-it-be

IMO, using const everywhere isn't a fruitful convention to have. Also, it's not a thing I consistently see being used in the ember community (open source addons, mainly).

Of course, you're this project maintainer. If this is a preference you want to see in this project, then please let me know and I'll change it.

amkirwan commented 5 years ago

@miguelcobain I'd prefer to stick with the eslint prefer-const rules where const is used for variables that are never reassigned

miguelcobain commented 5 years ago

@amkirwan I addressed all of your requests, I think. Let me know if you need anything else from me.

amkirwan commented 5 years ago

@miguelcobain I'll create a release with the updates shortly

amkirwan commented 5 years ago

thanks for the pr 👍

patocallaghan commented 5 years ago

Thanks for merging @amkirwan. Any idea when you'll be cutting a new release?

patocallaghan commented 5 years ago

👋 @amkirwan, any chance you could cut a new release with all the recent changes please? Thanks!

amkirwan commented 4 years ago

@miguelcobain @patocallaghan new release v2.0.5-beta