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
CB-13247 - Blog post for Cordova iOS 4.5.0 Release #727
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor suggestion -- otherwise LGTM!
www/_posts/2017-09-08-ios-release.md
Outdated
2. [CB-13164](https://issues.apache.org/jira/browse/CB-13164) - Integrated cordova-plugin-console to build in support for window.console. | ||
3. [CB-10916](https://issues.apache.org/jira/browse/CB-10916) - Support [display name](/docs/en/dev/config_ref/index.html#name) for **iOS** | ||
|
||
Note that for [CB-13164](https://issues.apache.org/jira/browse/CB-13164) if you already included `cordova-plugin-console` in your project, you must **remove** that plugin if not your project will not build. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps
Important! If you have included
cordova-plugin-console
in your project, you must remove it, otherwise your project will not build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, this needs to be called out more prominently
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it really not build? As this plugin is part of many, many projects (Ionic includes it by default) this would affect a lot of people.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it ends up with duplicate symbol linker errors:
duplicate symbol _OBJC_CLASS_$_CDVLogger in:
/Users/me/Library/Developer/Xcode/DerivedData/Project/Build/Intermediates/ArchiveIntermediates/Project/IntermediateBuildFilesPath/Project.build/Release-iphoneos/Project.build/Objects-normal/arm64/CDVLogger.o
platforms/ios/build/device/libCordova.a(CDVLogger.o)
duplicate symbol _OBJC_METACLASS_$_CDVLogger in:
/Users/me/Library/Developer/Xcode/DerivedData/Project/Build/Intermediates/ArchiveIntermediates/Project/IntermediateBuildFilesPath/Project.build/Release-iphoneos/Project.build/Objects-normal/arm64/CDVLogger.o
platforms/ios/build/device/libCordova.a(CDVLogger.o)
ld: 2 duplicate symbols for architecture arm64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks all -- I'll add the change to make it more prominent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes added.
www/_posts/2017-09-08-ios-release.md
Outdated
|
||
Note that for [CB-13164](https://issues.apache.org/jira/browse/CB-13164) if you already included `cordova-plugin-console` in your project, you must **remove** that plugin if not your project will not build. | ||
|
||
If you ever needed to disable the built in console plugin, comment out or remove the `Console` `<feature>` tag in your platform specific `config.xml`, and/or call this right after the `deviceready` event: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is a "platform specific config.xml
" - there is only one in my projects.
Does this refer to <platform name="ios">
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in platforms/platformName there is a platform specific config.xml for platformName
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that disabling the plugin will not get rid of the potential linker error conflict, it just disables the included plugin at runtime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, didn't know about these config.xml files. These never get modified normally, do they?
(There is this "meme" in lots of StackOverflow, forum and blog posts that you should remove the console plugin before building a production build because "security" or "performance")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are build artifacts. Unfortunately this instruction is the best we can do with the current way its implemented. There is no way to remove "private" plugins to a platform. Since we are moving to npm in the way the CLI does things, the plugins could be local to the platform folder, and we could add them as dependencies instead which is installed by the CLI. That would make it less coupled like it is right now.
I'll leave this here until Monday morning PDT (Tues midnight SGT here) when I will publish this with the updated docs, and tweet. |
Merging this in now, and generating the docs, then publishing. |
Now that it's published I noticed that it would have been good to include the CLI command to remove |
@janpio spin up a PR to add it. But the site won't likely be updated for some time so it may be stale by then (it takes over an hour to generate and deploy a site) |
Platforms affected
iOS
What does this PR do?
Blog post for cordova-ios@4.5.0 releaes
What testing has been done on this change?
Built dev version of docs and previewed.
Checklist