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

Add verbose option to version command #1307

Merged
merged 2 commits into from
Jul 14, 2014
Merged

Conversation

dschmidt
Copy link
Contributor

@dschmidt dschmidt commented Jul 8, 2014

Let me know if you want a test and how you would imagine the implementation.

@rwjblue
Copy link
Member

rwjblue commented Jul 8, 2014

Lookin' good! I'd like to see ember version spit out Ember CLI's version along with node version and npm version. Then have ember version --verbose include the rest of this info (which is also useful).

Can you add a changelog entry and some tests?


versions['npm'] = require('npm').version;

for (module in versions) {
Copy link
Member

Choose a reason for hiding this comment

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

Should be for (var module in versions) { (otherwise you are making a global variable and JSHint is not happy).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

woops, will fix

@dschmidt
Copy link
Contributor Author

dschmidt commented Jul 8, 2014

ChangeLog entry no problem.

Tests I'm not really sure what and how to test this thus I was asking :-)

@dschmidt
Copy link
Contributor Author

dschmidt commented Jul 8, 2014

Of course I can squash the commits once the PR is ready

@stefanpenner
Copy link
Contributor

Mind adding a quick test?

@gavacho
Copy link
Contributor

gavacho commented Jul 10, 2014

I tried adding a test for this but I'm not sure I'm doing it right. Here's my effort: gavacho@6125691

@dschmidt
Copy link
Contributor Author

Looking reasonable to me. Pulling it into my fork, so it shows up here and can be discussed 👍

@rwjblue
Copy link
Member

rwjblue commented Jul 14, 2014

Seems good. Can you squash into two commits (one for each author)?

versions['npm'] = require('npm').version;

for(var module in versions) {
switch(module) {
Copy link
Member

Choose a reason for hiding this comment

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

Might be able to remove this switch in favor of something like https://gist.github.com/rjackson/df7c8f0a2b74ee668778.

@rwjblue
Copy link
Member

rwjblue commented Jul 14, 2014

Will need another rebase (changelog has a conflict from a recent PR merged).

@rwjblue
Copy link
Member

rwjblue commented Jul 14, 2014

Looks like JSHint is not happy with the indentation of one of the }'s.

rwjblue added a commit that referenced this pull request Jul 14, 2014
Add verbose option to version command
@rwjblue rwjblue merged commit 6366875 into ember-cli:master Jul 14, 2014
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

4 participants