-
Notifications
You must be signed in to change notification settings - Fork 990
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-13164: Integrated cordova-plugin-console #330
Conversation
Not sure why AppVeyor is saying "AppVeyor was unable to build non-mergeable pull request", it looks mergeable to me. |
57e41a0
to
14b0490
Compare
I've rebased w/ master, let's see if that rejigs the wires. |
Codecov Report
@@ Coverage Diff @@
## master #330 +/- ##
=======================================
Coverage 63.95% 63.95%
=======================================
Files 14 14
Lines 1673 1673
Branches 277 277
=======================================
Hits 1070 1070
Misses 603 603 Continue to review full report at Codecov.
|
@@ -65,6 +65,10 @@ | |||
<feature name="LocalStorage"> | |||
<param name="ios-package" value="CDVLocalStorage"/> | |||
</feature> | |||
<feature name="Console"> |
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.
You need to add this in defaults.xml as well. I tried adding the platform to a newly created CLI project, console.log failed because this wasn't in config.xml.
See b0c71bf
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.
really.... my locally generated project worked fine without that....
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 is that a cordova-cli integration requirement or something? I was testing it without using cordova-cli, just by creating a project via ./bin/create
, etc.
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.
Yes, see the linked commit description.
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.
makes mental note as good use case for integration test
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.
rebased + added
…indow.console. - added native source files for CDVLogger - updated pbxproj file for CordovaLib to take the new files into account
14b0490
to
adabe28
Compare
What does this PR do?
Adds support for
window.console
to be built in to generated cordova-ios projects. Essentially: sunsetting cordova-plugin-console and integrating its iOS bits into cordova-ios.NOTE: this PR requires the use of latest master of cordova-js.
What testing has been done on this change?
Just
npm test
locally on my Mac. Unsure if I should add any additional tests? @shazron leaning on you for guidance on that question.Checklist