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.

Sign up for GitHub

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

Jump to bottom

Ember Cli 2.12 upgrade without bower dependency #16

Merged
merged 11 commits into from
Apr 20, 2017
Merged

Ember Cli 2.12 upgrade without bower dependency #16

merged 11 commits into from
Apr 20, 2017

Conversation

PoslinskiNet
Copy link
Owner

@PoslinskiNet PoslinskiNet commented Apr 11, 2017 •
edited
Loading

Issues

This PR will close:

Tasks:

  • ESlint
  • ES6
  • brocfile removal
  • FastBoot support
  • Update helpers readme if needed
  • Move from IntroJS via bower to introJS via npm
  • Readme transition guide
  • IntroJS license check & info
  • Release the current version (1.x)
  • Release 2.x
  • Publish 2.x via NPM

Transition guide (release change log)

The biggest change introduced in the version 2.0.0 is exchanging IntroJS bower dependency to NPM equivalent. The new version has support for FastBoot, and doesn't cause any deprecation errors when is used with the current version of Ember CLI.

To upgrade from version 1.x to 2.0.0 you should:

  1. If you are only using IntroJS via ember-introjs addon then run: 
    bower uninstall intro.js --save
    to remove bower dependency, and keep build directory clean, you can reinstall all bower packages with: 
    bower prune & bower install
  2. Update the current version of ember-introjs: 
    npm install ember-introjs --save-dev
  3. If for some reason, you would like to use IntroJS directly, keep in mind that it is not globally available anymore. You can add it with: 
    import introJs from 'ember-introjs/intro-js';

@PoslinskiNet PoslinskiNet requested a review from Exelord April 11, 2017 04:53
@PoslinskiNet PoslinskiNet force-pushed the feature-ember-cli-2.12-upgrade branch from 6f8625c to a0d40c2 Compare April 11, 2017 07:16
Exelord
Exelord requested changes Apr 11, 2017
View reviewed changes
addon/components/intro-js.js Outdated
@@ -25,9 +24,13 @@ var INTRO_JS_OPTIONS = [

var IntroJSComponent = Ember.Component.extend({

setupIntroJS: Ember.observer('start-if', function(){
// setupIntroJS: Ember.observer('start-if', function(){
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove comments

Copy link
Owner Author

Choose a reason for hiding this comment

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

Done

package.json Outdated
"version": "1.0.0",
"description": "The default blueprint for ember-cli addons.",
"version": "2.0.0",
"description": "Intro.js wrapper component",
Copy link
Collaborator

Choose a reason for hiding this comment

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

In GitHub description, we have: An Ember Component for intro.js Shouldn't we keep that unified?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Done

package.json Outdated
"keywords": [
"ember-addon"
],
"license": "MIT",
Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, with MIT but we should also warn somewhere users that Intro.js is on AGPL-3.0

package.json Outdated
"version": "2.0.0",
"description": "Intro.js wrapper component",
"keywords": [
"ember-addon"
Copy link
Collaborator

Choose a reason for hiding this comment

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

More meaningful keywords? :)

Copy link
Owner Author

Choose a reason for hiding this comment

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

Done

tests/dummy/app/tests/integration/helpers-test.js
@@ -1,42 +0,0 @@
import startApp from 'dummy/tests/helpers/start-app';
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should revert it

Copy link
Owner Author

Choose a reason for hiding this comment

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

Done

addon/helpers.js
@@ -1,75 +0,0 @@
import IntroJSComponent from 'ember-introjs/components/intro-js';
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should revert it

Copy link
Owner Author

Choose a reason for hiding this comment

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

Done

tests/test-helper.js
@@ -1,43 +1,4 @@
/* globals require, mocha */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you sure we should remove this test-helper file?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Reverted and moved

@PoslinskiNet PoslinskiNet force-pushed the feature-ember-cli-2.12-upgrade branch from b6c3aaf to 118f8db Compare April 11, 2017 14:55
@PoslinskiNet PoslinskiNet force-pushed the feature-ember-cli-2.12-upgrade branch from 118f8db to 7558068 Compare April 12, 2017 10:09
@PoslinskiNet
Copy link
Owner Author

@Exelord I've applied your suggestions and finished tasks. Do you have any further feedback?

@PoslinskiNet PoslinskiNet changed the title Ember Cli 2.12 upgrade Ember Cli 2.12 upgrade without bower dependency Apr 17, 2017
Exelord
Exelord approved these changes Apr 17, 2017
View reviewed changes
Copy link
Collaborator

@Exelord Exelord left a comment

Choose a reason for hiding this comment

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

Great! Looks good :)
Remember, before merging please release current master as a v.1.1.0 and then release those changes as a v2.0.0 👍 Don't forget about publishing on npm.

@Exelord
Copy link
Collaborator

Exelord commented Apr 17, 2017

@PoslinskiNet I have also some remarks to the Transition guide:

  1. You don't have to remove introjs from bower deps. You can. It's optional. It won't break the addon.
  2. This will not work as expected. It won't remove introjs from bower.json

Alternatively you can reinstall all bower packages with bower prune & bower install

  1. You can just change the version in package.json to 2.0.0 and then type npm i... there is no need to update the package. What it does is updating npm cache. So, if you are upgrading the package there is no cache :)
  2. You should warn! users that we removed introJs from the globals. Now you have to import it import introJs from 'intro-js. This is the very important change that makes this package incompatible with previous versions.

@PoslinskiNet
Copy link
Owner Author

@Exelord thanks for the catch. I've updated a guide a little bit.

@Exelord
Copy link
Collaborator

Exelord commented Apr 17, 2017

Nice :) So, now the upgrade path is:

  1. npm install ember-introjs --save-dev (it will install the latest version and modify package.json)
  2. bower uninstall intro.js --save (it will remove old intro.js dependency)
  3. import introJs from 'ember-introjs/intro-js' (import intro in places where it's needed)

@Exelord Exelord mentioned this pull request Apr 17, 2017
Closed
@PoslinskiNet PoslinskiNet merged commit 4379fc1 into master Apr 20, 2017
@PoslinskiNet PoslinskiNet deleted the feature-ember-cli-2.12-upgrade branch April 20, 2017 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Exelord Exelord Exelord approved these changes

Assignees

@Exelord Exelord

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@PoslinskiNet @Exelord