Skip to content

Revert Unsupported Commands back to Deprecated for 0.4.0 release#74

Merged
lukemelia merged 4 commits intoember-cli-deploy:masterfrom
danshultz:revert-unsupported-commands-to-deprecated
Mar 7, 2015
Merged

Revert Unsupported Commands back to Deprecated for 0.4.0 release#74
lukemelia merged 4 commits intoember-cli-deploy:masterfrom
danshultz:revert-unsupported-commands-to-deprecated

Conversation

@danshultz
Copy link
Copy Markdown
Contributor

This reverts the commit that moved the original ember-cli-deploy commands to unsupported for users coming to this project from the original ember-cli-deploy package. It also lists a few original ember-deploy commands as deprecated also.

@achambers thanks for deprecating and then un-supporting in 2 commits. Made this an easy item to knock out this morning.

This closes issue #72

@achambers
Copy link
Copy Markdown
Member

@danshultz Don't thank me yet. We need the original commands to actually work, which they don't currently with that DeprecatedCommand object. You'll see that it's overwriting the run command.

@danshultz
Copy link
Copy Markdown
Contributor Author

Ha, thanks, I'll wire that up now....

womp womp womp on my :)

@achambers
Copy link
Copy Markdown
Member

I added a couple of achambers/ember-cli-deploy specific commands there
too. Do we need to somehow replicate them, for the time being, for people
that might have upgraded from my repo to this one?

I am currently adding a post install warning to users installing v 0.4.0+
that look to have the config from my repo set ( it sure how else to tell if
they are upgrading from my repo). But I'm not sure that's enough. We don't
really any to break their experience either.
On Tue, 3 Mar 2015 at 12:56 Dan Shultz notifications@github.com wrote:

Ha, thanks, I'll wire that up now....

womp womp womp on my :)

Reply to this email directly or view it on GitHub
#74 (comment)
.

@danshultz
Copy link
Copy Markdown
Contributor Author

@achambers I'm using your readme still here --> https://www.npmjs.com/package/ember-cli-deploy although if you have your old repo, maybe put it up as ember-cli-deploy-old or something so I can have that as reference.

At the moment, I'm cross checking your readme to update the commands from the original ember-cli-deploy (deploy activate and deploy:versions). I'll have a PR momentarily with some added comments for you to review

@achambers
Copy link
Copy Markdown
Member

@danshultz https://github.com/achambers/ember-cli-deploy-original

I'm about to take lunch shortly so can chat about things on slack if you
want.
On Tue, 3 Mar 2015 at 13:10 Dan Shultz notifications@github.com wrote:

@achambers https://github.com/achambers I'm using your readme still
here --> https://www.npmjs.com/package/ember-cli-deploy although if you
have your old repo, maybe put it up as ember-cli-deploy-old or something so
I can have that as reference.

At the moment, I'm cross checking your readme to update the commands from
the original ember-cli-deploy (deploy activate and deploy:versions). I'll
have a PR momentarily with some added comments for you to review

Reply to this email directly or view it on GitHub
#74 (comment)
.

support `ember deploy:versions` and `ember activate` for release 0.4.0 to support users on the original ember-cli-deploy npm package.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@achambers - I used the new location of config files, I am leaving the notification to users of the original ember-cli-deploy package to a later PR.

@danshultz
Copy link
Copy Markdown
Contributor Author

I should be around we can chat in slack - couple of questions this PR opens up...

  1. Users who update will need to install the ember-deploy-s3 plugin as it looks to be that deploying to s3 was included in the original ember-cli-deploy package.
  2. Users will most likely have https://github.com/achambers/ember-cli-deploy-redis-index-adapter installed. We'll need to verify that it still works with 0.4.0 if we are aiming not to break them.

I figure we might handle that in the upgrade notes? If we are however breaking those users, should we leave the ember deploy:versions and ember activate commands as "unsupported"?

@achambers
Copy link
Copy Markdown
Member

Yo @danshultz . Where does this PR currently sit? Is this good to go for 0.4.0?

@danshultz
Copy link
Copy Markdown
Contributor Author

This PR is working in its entirety.

This PR re-enables 'deploy: index', 'deploy: assets' and adds support for 'deploy: versions' and 'activate' which provides command compatibility with previous versions of ember deploy and ember cli deploy.

@lukemelia mentioned removing the deprecation of the deploy: index and deploy: asset command but thinking it through, we did want to remove these commands in favor of using 'deploy' which does the build and deploying all assets and the index.

I'm out at the moment but if others can chime in on their opinions I can make any changes to this PR today. Otherwise I think this is ok to merge unless anyone thinks we should do something different.

@lukemelia
Copy link
Copy Markdown
Contributor

My primary concern here is that I don't think deploy:activate should be deprecated, and certainly not before we have another way for someone to deploy without immediately activating the new version. @achambers @danshultz Agree/disagree?

@danshultz
Copy link
Copy Markdown
Contributor Author

Ah, I see the confusion. deploy: activate is not deprecated but the bare 'ember activate' is. This was a command from 0.6.0

@lukemelia
Copy link
Copy Markdown
Contributor

Ah, got it. In that case, +1 to merge.

lukemelia added a commit that referenced this pull request Mar 7, 2015
…deprecated

Revert Unsupported Commands back to Deprecated for 0.4.0 release
@lukemelia lukemelia merged commit ddd28f5 into ember-cli-deploy:master Mar 7, 2015
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