You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by joshdholtz <gi...@git.apache.org> on 2017/06/28 20:11:32 UTC
[GitHub] cordova-ios pull request #324: CB-12966: (ios) Fix bug by escaping project n...
GitHub user joshdholtz opened a pull request:
https://github.com/apache/cordova-ios/pull/324
CB-12966: (ios) Fix bug by escaping project name in podfile template
### Platforms affected
- iOS
### What does this PR do?
- Fixes a bug when the project name has a single quote in it that caused the `Podfile` to have invalid syntax
### What testing has been done on this change?
- Just manual testing on my own project. I am not familiar with how to write a proper tests for this change π I know I need to add something in `Podfile.spec.js` but not exactly sure what/how exactly I need to do this. Any suggestions on how to write these tests would be truly appreciated β€οΈ
### Checklist
- [x] [Reported an issue](http://cordova.apache.org/contribute/issues.html) in the JIRA database
- [x] Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
- [ ] Added automated test coverage as appropriate for this change.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/joshdholtz/cordova-ios master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-ios/pull/324.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #324
----
commit ff8061e4de993ec5104701a7211a57b774a9fb74
Author: Josh Holtz <jo...@rokkincat.com>
Date: 2017-06-28T17:22:27Z
CB-12966: (ios) Fix bug by escaping project name in podfile template
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-ios issue #324: CB-12966: (ios) Fix bug by escaping project name in ...
Posted by joshdholtz <gi...@git.apache.org>.
Github user joshdholtz commented on the issue:
https://github.com/apache/cordova-ios/pull/324
@shazron Rebased from master so that the tests and linter actually run and pass now. Also separated out the escaping of single quotes into its own function for better testing and added a tests for that new function. Lemme know if there is anything I need to change to make these changes fit better with the style of this project π
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-ios issue #324: CB-12966: (ios) Fix bug by escaping project name in ...
Posted by shazron <gi...@git.apache.org>.
Github user shazron commented on the issue:
https://github.com/apache/cordova-ios/pull/324
We had to pull in new code for the new linter using eslint. Consequently, some files have changed. Can you try rebasing, then running `npm run eslint` to make sure it's all kosher
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-ios issue #324: CB-12966: (ios) Fix bug by escaping project name in ...
Posted by joshdholtz <gi...@git.apache.org>.
Github user joshdholtz commented on the issue:
https://github.com/apache/cordova-ios/pull/324
@renatoalencar I have no idea why the tests are not passing. It seems like other tests aren't passing either. You can use this by doing something like...
```
ionic platform add https://github.com/joshdholtz/cordova-ios.git
```
or
```
cordova platform add https://github.com/joshdholtz/cordova-ios.git
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-ios issue #324: CB-12966: (ios) Fix bug by escaping project name in ...
Posted by joshdholtz <gi...@git.apache.org>.
Github user joshdholtz commented on the issue:
https://github.com/apache/cordova-ios/pull/324
@shazron Rebased and updated coding styles π
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-ios issue #324: CB-12966: (ios) Fix bug by escaping project name in ...
Posted by renatoalencar <gi...@git.apache.org>.
Github user renatoalencar commented on the issue:
https://github.com/apache/cordova-ios/pull/324
I really need this bug fix, did you understand why this is not passing in tests?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-ios issue #324: CB-12966: (ios) Fix bug by escaping project name in ...
Posted by shazron <gi...@git.apache.org>.
Github user shazron commented on the issue:
https://github.com/apache/cordova-ios/pull/324
Not passing the linter:
```
bin\templates\scripts\cordova\lib\Podfile.js: line 93, col 51, Mixed double and single quotes.
bin\templates\scripts\cordova\lib\Podfile.js: line 93, col 59, Mixed double and single quotes.
```
Basically the style for this repo is strings are single quoted. So you would reverse your escaping in line 93
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-ios issue #324: CB-12966: (ios) Fix bug by escaping project name in ...
Posted by codecov-io <gi...@git.apache.org>.
Github user codecov-io commented on the issue:
https://github.com/apache/cordova-ios/pull/324
# [Codecov](https://codecov.io/gh/apache/cordova-ios/pull/324?src=pr&el=h1) Report
> Merging [#324](https://codecov.io/gh/apache/cordova-ios/pull/324?src=pr&el=desc) into [master](https://codecov.io/gh/apache/cordova-ios/commit/26beb94e2d454917db0bb024ab75e8dd9759c028?src=pr&el=desc) will **increase** coverage by `0.02%`.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/cordova-ios/pull/324?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #324 +/- ##
==========================================
+ Coverage 63.77% 63.79% +0.02%
==========================================
Files 14 14
Lines 1623 1624 +1
Branches 277 277
==========================================
+ Hits 1035 1036 +1
Misses 588 588
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-ios/pull/324?src=pr&el=tree) | Coverage Ξ | |
|---|---|---|
| [bin/templates/scripts/cordova/lib/Podfile.js](https://codecov.io/gh/apache/cordova-ios/pull/324?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9zY3JpcHRzL2NvcmRvdmEvbGliL1BvZGZpbGUuanM=) | `75.86% <100%> (+0.2%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-ios/pull/324?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Ξ = absolute <relative> (impact)`, `ΓΈ = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-ios/pull/324?src=pr&el=footer). Last update [26beb94...c66b4ba](https://codecov.io/gh/apache/cordova-ios/pull/324?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-ios pull request #324: CB-12966: (ios) Fix bug by escaping project n...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-ios/pull/324
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org