Skip to content
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

Fixing generation failures due to asset catalog & **/*.png glob patterns handling #346

Merged
merged 2 commits into from May 11, 2019

Conversation

kwridan
Copy link
Collaborator

@kwridan kwridan commented May 10, 2019

Resolves #345

Short description 📝

  • Using the **/* style patterns are "eager" they will list out files recursively including ones within .xcassets
  • The order in which the ProjectFileElements elements are processes isn't guaranteed
  • If a file within the .xcassets is encountered first, e.g. App/Resources/Assets.xcassets/assetCatalogLogo.imageset/tuist.png before App/Resources/Assets.xcassets, App/Resources/Assets.xcassets is mistaken for a group and isn't added as a file element

Solution 📦

  • To mitigate this, the logic that determines if a group or file is added checks for .xcassets files

Implementation 👩‍💻👨‍💻

Detail in a checklist the steps that you took to implement the PR.

  • Update fixtures
  • Update unit tests
  • Add logic to check for .xcassets
  • Update changelog

Test Plan 🛠

  • Verify unit tests pass
  • Verify acceptance tests pass
  • Run tuist generate within fixtures/ios_app_with_framework_and_resources
  • Verify the generation passes
  • Verify the asset catalog is included in the generated MainApp within the workspace

…rns handling

Resolves tuist#345

- Using the `**/*` style patterns are "eager" they will list out files recursively including ones within `.xcassets`
- The order in which the `ProjectFileElements` elements are processes isn't guaranteed
- If a file within the `.xcassets` is encountered first, e.g. `App/Resources/Assets.xcassets/assetCatalogLogo.imageset/tuist.png` before `App/Resources/Assets.xcassets`, `App/Resources/Assets.xcassets` is mistaken for a group and isn't added as a file element
- To mitigate this, the logic that determines if a group or file is added checks for `.xcassets` files

Test Plan:

- Verify unit tests pass
- Verify acceptance tests pass

- Run `tuist generate` within `fixtures/ios_app_with_framework_and_resources`
- Verify the generation passes
- Verify the asset catalog is included in the generated `MainApp` within the workspace
@tuistbot
Copy link
Contributor

1 Warning
⚠️ Have you introduced any user-facing changes? If so, please take some time to update the documentation. Keeping the documentation up to date makes it easier for users to learn how to use Tuist.

Generated by 🚫 Danger

@codecov
Copy link

codecov bot commented May 10, 2019

Codecov Report

Merging #346 into master will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #346      +/-   ##
=========================================
+ Coverage   90.66%   90.7%   +0.03%     
=========================================
  Files         281     281              
  Lines       12982   13026      +44     
=========================================
+ Hits        11770   11815      +45     
+ Misses       1212    1211       -1
Impacted Files Coverage Δ
...TuistGenerator/Generator/ProjectFileElements.swift 95.54% <100%> (+0.02%) ⬆️
...atorTests/Generator/ProjectFileElementsTests.swift 99.77% <100%> (+0.02%) ⬆️
Sources/TuistGenerator/Graph/Graph.swift 91.44% <0%> (+0.45%) ⬆️

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 e562c49...d9fca53. Read the comment docs.

@pepicrft
Copy link
Contributor

Ignore the CI checks from Netlify that are failing

@kwridan kwridan merged commit 7935c55 into tuist:master May 11, 2019
@kwridan kwridan deleted the fixes/xcassets branch May 11, 2019 06:29
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.

Projects with asset catalogs may fail to generate when using **/*.png resource glob patterns
3 participants