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

[utils] Adjust logic in clear-container-cache. factoryCache -> factoryManagerCache #51

Conversation

alexhancock
Copy link
Collaborator

This change should fix things related to #48 for now, at least based on testing I have done with a vanilla CLI app.

We should all organize and have a discussion about the future of Ember's component and component manager interfaces so that eventually this addon can rely on public APIs instead of private ones. cc @toranb @MiguelMadero

See comments here for more on this topic.

Also: emberjs/ember.js#15057

@alexhancock
Copy link
Collaborator Author

alexhancock commented Dec 11, 2017

Looks like the tests have failed. Is there a good way I can run them locally to fix things up?

@toranb
Copy link
Collaborator

toranb commented Dec 11, 2017

@alexhancock I've thrown up another PR that solves this build issue for everything except ember 1.13 (ember-try issue atm). Asked @MiguelMadero to give it a look before we merge tomorrow

@toranb
Copy link
Collaborator

toranb commented Dec 11, 2017

@alexhancock huge huge thank you! Not sure why I never tried the cache factory clear like you did here! So thankful for your time and involvement today! I'll likely close out this PR (if that's cool w/ you) and ask that either you take credit in PR #52 or re-do that PR (after we make a decision on the ember 1.13 ember-try config). I'm not looking for credit/glory here but do think we need that ember-cli upgrade

Just let me know how you want to proceed later in the week

@alexhancock
Copy link
Collaborator Author

alexhancock commented Dec 11, 2017

@toranb No problem! Happy to just close this one in favor of that! In terms of credit, maybe you could just @ mention me in the commit message for b53977c, but anything is fine really.

I will keep an eye on that change to see what the fix for the ENV issues ends up being.

Thanks!

@alexhancock
Copy link
Collaborator Author

Closing in favor of #52

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

2 participants