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 back in build tool default loading order rules #340

Merged
merged 3 commits into from Apr 4, 2016

Conversation

Projects
None yet
3 participants
@skirunman
Contributor

skirunman commented Apr 4, 2016

Thanks for submitting a PR! We'll try to look at it as soon as possible.

If you are adding significant new content, please take a moment to include an update to the changelog in your PR.

@skirunman

This comment has been minimized.

Contributor

skirunman commented Apr 4, 2016

This should fix #336

skirunman added some commits Apr 4, 2016

@skirunman

This comment has been minimized.

Contributor

skirunman commented Apr 4, 2016

This includes some cross reference changes to mobile.md. Probably need to confirm with @martijnwalraven that the cordova-build-override/ directory still works.

@stubailo

This comment has been minimized.

Contributor

stubailo commented Apr 4, 2016

Yeah this looks great to me. I expect the build override still works, since that would be a big backcompat change and wasn't mentioned in http://guide.meteor.com/1.3-migration.html#breaking-changes-mobile

@stubailo stubailo merged commit b4ce2af into meteor:master Apr 4, 2016

2 checks passed

CLA Author has signed the Meteor CLA.
Details
ci/circleci Your tests passed on CircleCI!
Details
@stubailo

This comment has been minimized.

Contributor

stubailo commented Apr 4, 2016

Awesome work, this is a great new addition to the guide!

@martijnwalraven

This comment has been minimized.

Contributor

martijnwalraven commented Apr 4, 2016

The cordova-build-override mechanism still works, but I didn't include it in the guide because I don't think it is ever a good solution and it can lead to a lot of trouble.

The problem is that is overrides complete files, so if you use it to write your own config.xml for example, you lose all configuration information set by the build process and by installed plugins, which will likely break your app.

Cordova has a <config-file> tag as part of plugin.xml that allows you to selectively change parts of configuration files, including config.xml, so if you absolutely must make changes, the recommendation would be to write a dummy plugin. If we are including advanced build customization in the guide, I think we should mention that instead.

@stubailo

This comment has been minimized.

Contributor

stubailo commented Apr 4, 2016

@martijnwalraven OK, sorry I merged that if it isn't recommended! Please change it back if you think it is best.

@martijnwalraven

This comment has been minimized.

Contributor

martijnwalraven commented Apr 4, 2016

@stubailo: I've removed the recommendation to override config.xml and mentioned the plugin alternative in 4397c43.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment