Skip to content

Conversation

ksplache
Copy link
Contributor

No description provided.

@ksplache ksplache mentioned this pull request Oct 27, 2017
@jaimecbernardo
Copy link
Member

Hi @ksplache .

Thank you for the sample. I've been able to build and run it. I've pushed a commit to another repo with some suggested changes:

  • npm run setup-cordova assumes ./cordova exists. Added mkdir -p ./cordova && to the npm script.
  • Added the following build and run prerequisites to README.md : bower, gulp-cli and ios-deploy.
  • Added a cordova-plugin-console warning message disclaimer to README.md, to explain why the plugin install skip message that appears on newer versions of cordova can be ignored.

Here's the commit: jaimecbernardo@ec6e261

If these changes look good to you, would you mind integrating them into your branch or would you prefer that I apply them after merging this PR?

@ksplache
Copy link
Contributor Author

Thanks @jaimecbernardo - I'll go ahead and make those changes on my branch so that they come in with this PR.

@jaimecbernardo jaimecbernardo self-requested a review October 30, 2017 18:03
Copy link
Member

@jaimecbernardo jaimecbernardo left a comment

Choose a reason for hiding this comment

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

Thanks for the changes. I'll squash the commits and merge the PR.

@jaimecbernardo jaimecbernardo merged commit 113959e into JaneaSystems:master Oct 30, 2017
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.

2 participants