-
Notifications
You must be signed in to change notification settings - Fork 40
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
fix: resolve correct path to app info plist when multiple plist files are present (#144) #148
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.
Code wise LGTM 👍
Looks like a test file though might need to be updated. This line specifically will need the change: const expectedPlistPath = `${projName}-Info.plist`; |
Interesting that the ubuntu tests failed but windows & osx passed. |
I don't see any passing tests. Two test, ubuntu, had failed and caused GitHub Actions to trigger the cancel procedure of the other four remaining tests with out either starting or finishing. |
ah, yes, I misread the cancelled as passed (no big red X) will run/fix the tests locally and update my branch. |
… files are present (apache#144). When multiple plist files exists in a cordova-ios project (e.g. due to a plugin containing `<podspec>`), ConfigFile was updated under CB-5989 to select the app plist as the target for changes destined for *-Info.plist. However, the change made under CB-5989 incorrectly constructed the path to the app plist by omitting the project name subdirectory from the path, causing the fix to fail to work. This commit fixes this by correcting the constructed path to the app plist.
Codecov Report
@@ Coverage Diff @@
## master #148 +/- ##
==========================================
+ Coverage 88.23% 88.24% +0.01%
==========================================
Files 20 20
Lines 1173 1174 +1
==========================================
+ Hits 1035 1036 +1
Misses 138 138
Continue to review full report at Codecov.
|
Platforms affected
iOS
Motivation and Context
When multiple plist files exists in a cordova-ios project (e.g. due to a plugin containing
<podspec>
), ConfigFile was updated under CB-5989 to select the app plist as the target for changes destined for *-Info.plist.However, the change made under CB-5989 incorrectly constructed the path to the app plist by omitting the project name subdirectory from the path, causing the fix to fail to work.
This bug is outlined in #144.
Description
This commit fixes this by correcting the constructed path to the app plist.
Testing
I've created a repo containing a test Cordova project which reproduces the various scenarios to illustrate the issue:
https://github.com/dpa99c/cordova-common-issue-144
Details of these scenarios can be found in this comment.
Checklist
(platform)
if this change only applies to one platform (e.g.(android)
)