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

Remove unused dependencies #8

Merged
merged 13 commits into from
Jun 9, 2021

Conversation

nelstrom
Copy link
Contributor

@nelstrom nelstrom commented Jun 3, 2021

This addon was looking quite out of date. In particular, the fact that it depended on ember-cli-babel version 5 was starting to cause issues for an Ember app that depends on this addon.

These changes are fairly substantial, so I bumped the version from 0.2.0 to 1.0.0. I don't actually believe that there were any breaking changes, so maybe 0.3.0 would have been appropriate?

Here's a summary of what I changed:

Consolidating dependencies

The main functionality of this addon is implemented in the index.js file. That file has exactly two dependencies:

  • 'git-repo-info'
  • 'ember-cli-deploy-plugin'

It makes sense to declare these as dependencies in the package.json file. That leaves the following as devDependencies:

  • 'chai-as-promised'
  • 'chai'
  • 'ember-cli' (required from ember-cli-build.js)
  • 'glob'
  • 'mocha-jshint'
  • 'mocha'

These are mostly used for running unit tests.

Everything else must go! 🔥🔥🔥

Use native Promise instead of RSVP

Node has had Promise API for ages. This addon supports node 10 and later. We don't need RSVP as a polyfill anymore.

Update Travis configuration

The travis configuration was out of date (using node version 4!). I used the current version of ember-cli (3.26) to generate a scratch addon and copied the .travis.yml over to this addon, then made tweaks to get it working again.

Remove ember-try

ember-try is great for testing addons against different versions of Ember. To use ember try, you have to run tests via ember test, which in turn requires ember-source, ember-cli-htmlbars and ember-cli-babel (Remember? That was how we got here in the first place!)... I don't think any of that stuff belongs in this addon.

I removed the ember-try addon from package.json and also removed the config/ember-try.js file.

Remove bower

bower looks like a relic from a bygone era. We don't need it.

@nelstrom nelstrom force-pushed the remove-unused-dependencies branch from 220d2b9 to 62e9e25 Compare June 3, 2021 12:54
This is the version that is generated
when running `ember addon NAME`.
The unit tests already run the linter.
We don't need to run `ember test`, which requires ember-cli-babel and
ember-source and loads of other stuff.

We can just use `yarn test` which is lightweight
and appropriate for this addon.
@nelstrom nelstrom force-pushed the remove-unused-dependencies branch from 6f12b98 to 52e6c1b Compare June 3, 2021 13:09
This file is for configuring https://github.com/shipshapecode/ember-cli-release

The addon is deprecated.

The README recommends using https://github.com/release-it/release-it
This addon doesn't have any ember specific code in it,
so we don't really need to test this addon against different
versions of Ember.
@nelstrom nelstrom marked this pull request as ready for review June 3, 2021 13:44
@achambers
Copy link
Owner

Thanks for this @nelstrom . You're a ⭐

Would you mind dropping the commit that bumps the version then one that reverts that commit? No point them being there. Other than that, It's good to go.

@nelstrom nelstrom force-pushed the remove-unused-dependencies branch from 5265859 to 45e8358 Compare June 7, 2021 10:14
@nelstrom
Copy link
Contributor Author

nelstrom commented Jun 7, 2021

@achambers done.

It seems as though ember-cli makes it a hard requirement
that ember-cli-babel is available as a preprocessor for .js files.

https://github.com/ember-cli/ember-cli/blob/70ae16c5eb972076580b23e9c22579234749862e/lib/models/addon.js#L1308-L1334
"ember-cli-inject-live-reload": "^1.4.0",
"ember-cli-jshint": "^1.0.0",
"ember-cli-qunit": "^2.1.0",
"ember-cli-release": "1.0.0-beta.2",
Copy link
Owner

Choose a reason for hiding this comment

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

FYI - this addon (and the corresponding config/release.js file) is used for publishing this addon to npm and updating GH releases. However, https://github.com/release-it/release-it is the more modern way of doing this these days and so this repo could probably do with updating to use that. So, we can keep this change for now. Just an FYI.

@achambers achambers merged commit ae2a498 into achambers:master Jun 9, 2021
@achambers achambers deleted the remove-unused-dependencies branch June 9, 2021 13:57
@achambers
Copy link
Owner

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.

None yet

2 participants