cordova-ios
cordova-ios copied to clipboard
[minor] fix *-Info.plist & config.xml functionality
This is proposed to include and supersede PR #765, as a solution for #764, with some additional fixes and test updates that I think should be included at the same time.
- updates and improvements to unit testing of projectFile parse method, now with xcworkspace included in the "ios-config-xml" fixture
- include all changes from PR #765 so far, with possible solution to issue #764
- some followup fixes to PR #765
- add test cases for the code updated in PR #765
- some more general improvements
This proposal should be considered breaking since it now depends on existence of xcworkspace with the correct name. This should have been present in generated platforms/ios project for years.
I have discussed some other related issues with plists in #793, not sure whether or not this proposal would resolve any such related issues.
I would like to have multiple reviews before getting this proposal merged. I think it should be good to merge this with a squash merge, with all co-authored-by credits that I gave in 8dfeedc8f754f7638b96b6677a0f34a4d246c8c5, maybe with a rebase to cleanup some of the items that will go into the commit message.
closes #765 (PR #765) resolves #764
/cc @leogoesger
Codecov Report
Merging #795 into master will increase coverage by
0.11%
. The diff coverage is100%
.
@@ Coverage Diff @@
## master #795 +/- ##
==========================================
+ Coverage 74.36% 74.47% +0.11%
==========================================
Files 13 13
Lines 1845 1853 +8
==========================================
+ Hits 1372 1380 +8
Misses 473 473
Impacted Files | Coverage Δ | |
---|---|---|
bin/templates/scripts/cordova/lib/projectFile.js | 95.31% <100%> (+0.66%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 4ac1552...b2496e3. Read the comment docs.
@brodybits Is this ticket fixing a bug and also considered to be breaking because you are searching for xcworkspace
?
What if you change the xcworkspace
search to xcodeproj
?
Would this still fix the issue and change the PR's type from breaking to patch or minor?
Of corse the conflict will also need to be corrected.
There is a patch release (6.0.1
) being prepared. If this PR is resolving a bug and can be altered to be a patch instead of breaking, you might be able to get this PR in.
You could still create a second breaking PR after this is merged into master to change only the xcodeproj
to xcworkspace
...
The good news is that I would no longer consider this to be a breaking change. If I would remove the xcworkspace file and then build from the command line, it results in an error from xcodebuild. (I was able to remove the xcworkspace file and then build & run from Xcode, which I think is a side point.)
I do think this is too much change for a patch, though, should probably target a minor release.
I did try merging. The changes did not seem to work after merging, and some of the tests need rework due to removing shelljs. I think we should really rebase and rework. I will try to fit this in as soon as I can.
I now wonder if <apache/cordova-common/pull/148> may be related
@brodybits I can confirm that the PR you mentioned does not fix this issue, as I am still seeing this in [email protected]
. Though the cause of the issues is the same (having multiple Info.plist
) files, their fixes are separate.