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

Add Environment.namespaceToName() method #34

Merged
merged 1 commit into from
Apr 5, 2015
Merged

Add Environment.namespaceToName() method #34

merged 1 commit into from
Apr 5, 2015

Conversation

arthurvr
Copy link
Member

@arthurvr arthurvr commented Apr 3, 2015

Fix #20

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 91.88% when pulling 4b0548a on arthurvr:namespaceToName into c12276e on yeoman:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 91.88% when pulling 4b0548a on arthurvr:namespaceToName into c12276e on yeoman:master.

@SBoudrias
Copy link
Member

Should this be a static method or an instance method?

@SBoudrias
Copy link
Member

Sorry for the troll...

Anyway, we should double check where the call site for a similar function would be and see if it's more convenient to have it as an instance method rather than a static one.

@arthurvr
Copy link
Member Author

arthurvr commented Apr 4, 2015

Anyway, we should double check where the call site for a similar function would be and see if it's more convenient to have it as an instance method rather than a static one.

I guess it makes more sense as an instance method. I added a static one for now as that seemed what the original issue's title suggests, but am fine with it being an instance method. What would be the most handy in yo's case?

@SBoudrias
Copy link
Member

Looks okay to keep this as a static method.

SBoudrias
SBoudrias reviewed Apr 5, 2015
View reviewed changes
lib/environment.js
@@ -452,6 +452,17 @@ Environment.createEnv = function (args, opts, adapter) {
};

/**
* Convert a generators namespace to its name
*
* @param {Namespace} namespace
Copy link
Member

Choose a reason for hiding this comment

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

Namespace is not a valid type, we want a string here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Haha whoops! Updated.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 91.88% when pulling db159ff on arthurvr:namespaceToName into 3455871 on yeoman:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 91.88% when pulling db159ff on arthurvr:namespaceToName into 3455871 on yeoman:master.

@SBoudrias
Copy link
Member

Awesome, thanks!

SBoudrias added a commit that referenced this pull request Apr 5, 2015
@SBoudrias
Merge pull request #34 from arthurvr/namespaceToName
29d3eda 
Add `Environment.namespaceToName()` method
@SBoudrias SBoudrias merged commit 29d3eda into yeoman:master Apr 5, 2015
@arthurvr arthurvr deleted the namespaceToName branch April 5, 2015 21:12
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
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Environment.namespaceToName()
3 participants
@arthurvr @coveralls @SBoudrias