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

Fix generate command when both usePods option and --pod argument is used #7164

Merged
merged 1 commit into from Nov 7, 2017

Conversation

emrekutlu
Copy link
Contributor

@emrekutlu emrekutlu commented Jun 29, 2017

Resolves #7163

@rwjblue
Copy link
Member

rwjblue commented Jun 29, 2017

I agree that this is confusing, but the intent here was that --pod flag negates the value of usePods. So if you had usePods as true you would use --pod to get non-pods.

@emrekutlu
Copy link
Contributor Author

Hmm, so what does --classic do?

@Turbo87 Turbo87 changed the title [BUGFIX] fixes generate command when both usePods option and --p… Fix generate command when both usePods option and --pod argument is used Jul 23, 2017
Copy link
Member

@Turbo87 Turbo87 left a comment

Choose a reason for hiding this comment

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

@rwjblue I tend to agree with this patch. Having --pod disable pod style when --classic also exists seems very unintuitive.

@Turbo87 Turbo87 added the bug label Jul 23, 2017
@rwjblue
Copy link
Member

rwjblue commented Jul 24, 2017

@Turbo87 - This patch has nothing to do with using both --pod and --classic at the same time, what am I missing?

@hjdivad
Copy link
Contributor

hjdivad commented Jul 24, 2017

@Turbo87 - This patch has nothing to do with using both --pod and --classic at the same time, what am I missing?

I think the point is that the existence of --classic implies the semantics of --pod is --use-pods and not --toggle-pods and not (only) that they're confusing when used together

@rwjblue
Copy link
Member

rwjblue commented Jul 25, 2017

I totally agree with you there, but this change would be a breaking change for anyone leveraging the existing behavior.

I'd be totally fine with adding a deprecation / warning for anyone that has both usePods and specifies either --pods or --classic specified, but not with arbitrarily changing the meaning of the flags (at least not within the same release, but it could be feathered out)...

@Turbo87
Copy link
Member

Turbo87 commented Jul 25, 2017

@emrekutlu can you prepare a PR that prints a warning to the console when --pod is used in combination with usePods?

@rwjblue
Copy link
Member

rwjblue commented Nov 5, 2017

@emrekutlu - Ping?

@emrekutlu
Copy link
Contributor Author

@rwjblue sorry for the wait, I am going to update the pull request this week hopefully. Thanks for pinging.

@emrekutlu
Copy link
Contributor Author

@rwjblue I updated the pull request.

@rwjblue rwjblue merged commit b7828db into ember-cli:master Nov 7, 2017
@rwjblue
Copy link
Member

rwjblue commented Nov 7, 2017

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants