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

Move "ember-cli" version printing into "ember --version" command #5655

Merged
merged 2 commits into from
Mar 22, 2016
Merged

Move "ember-cli" version printing into "ember --version" command #5655

merged 2 commits into from
Mar 22, 2016

Conversation

Turbo87
Copy link
Member

@Turbo87 Turbo87 commented Mar 21, 2016

@kellyselden
Copy link
Member

This removes the version printing from every command? I believe this is desired.

@stefanpenner
Copy link
Contributor

I think we can remove this, we used to have it to prevent common confusions. But maybe we can get rid of it...

@rwjblue
Copy link
Member

rwjblue commented Mar 21, 2016

FWIW, as it stands today this console output is confusing (version: 2.4.2) doesn't tell me really anything (is this the current version? of what thing? Ember? EmberCLI? etc). I'm definitely in favor of removing this on every command...

@Turbo87
Copy link
Member Author

Turbo87 commented Mar 21, 2016

@rwjblue I've changed it to output ember-cli: 2.4.2 on ember --version to be more explicit

@Turbo87 Turbo87 force-pushed the version-output branch 2 times, most recently from 2edec5e to e84ea4e Compare March 21, 2016 18:45
kellyselden
kellyselden reviewed Mar 21, 2016
View reviewed changes
tests/unit/cli/cli-test.js
@@ -79,10 +79,6 @@ afterEach(function() {
commands = argv = ui = undefined;
});

function assertVersion(string, message) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think the version output is ever tested anymore after removing this, because I expected to see 'version: ' => 'ember-cli: ' in the diff.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed with 16d998f

@Turbo87
Copy link
Member Author

Turbo87 commented Mar 22, 2016

@kellyselden @stefanpenner r?

@kellyselden
Copy link
Member

@homu r+

@kellyselden
Copy link
Member

homu is thinking hard again.

@kellyselden
Copy link
Member

@homu r+

@kellyselden
Copy link
Member

@homu r+

@kellyselden
Copy link
Member

Well dang, I guess we merge the old fashioned way once CI finishes

stefanpenner added a commit that referenced this pull request Mar 22, 2016
@stefanpenner
Merge pull request #5655 from Turbo87/version-output
1ac9139 
Move "ember-cli" version printing into "ember --version" command
@stefanpenner stefanpenner merged commit 1ac9139 into ember-cli:master Mar 22, 2016
@Turbo87 Turbo87 deleted the version-output branch March 24, 2016 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
internal
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Turbo87 @kellyselden @stefanpenner @rwjblue