feat(launchpad): Update artifact factory logic to cover more cases#140
feat(launchpad): Update artifact factory logic to cover more cases#140NicoHinderling wants to merge 2 commits intomainfrom
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #140 +/- ##
==========================================
+ Coverage 71.63% 71.78% +0.14%
==========================================
Files 93 93
Lines 7320 7365 +45
Branches 860 863 +3
==========================================
+ Hits 5244 5287 +43
Misses 1724 1724
- Partials 352 354 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| # Format 1: MyApp.xcarchive/Products/**/*.app | ||
| "*.xcarchive/Products/**/*.app", | ||
| # Format 2: Products/Applications/*.app (direct extraction) | ||
| "Products/Applications/*.app", |
There was a problem hiding this comment.
Is this something we want to support? I'm a bit confused with these two cases, why wouldn't there be a top-level .xcarchive wrapping these folders?
There was a problem hiding this comment.
+1 I don't think we should support this, or the one below it. We did in emerge, but now we are normalizing all the artifacts (in the CLI) and we should expect everything to have a well defined structure by this point. This will help us reduce a lot of the bugs we saw with bespoke folder structures in emerge
| app_bundle_path = app_bundle_path.relative_to(xcarchive_dir) | ||
| file_path = xcarchive_dir / "ParsedAssets" / app_bundle_path / json_name | ||
| else: | ||
| # Format 2: Products/Applications/... (direct extraction) |
There was a problem hiding this comment.
Same as above, I don't think we should support this case
|
ended up fixing this bug in the CLI instead |

When testing on a variety of different xcarchives, I found the existing logic failing to properly identify that it was an xcarchive. These changes fixed all the cases I found