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

Support Multiple Header Paths #459

Merged
merged 10 commits into from Jul 25, 2019

Conversation

adamkhazi
Copy link
Contributor

@adamkhazi adamkhazi commented Jul 24, 2019

Resolves #354

Short description 📝

Add support to specify array of paths / glob patterns for headers such as the case below:

Headers(public: ["Sources/A/**/*.h", "Sources/B/**/*.h"],
        private: ["Sources/C/**/*.h", "Sources/D/**/*.h"],
        project: "Sources/E/**/*.h")

Solution 📦

Similar to the way multiple source file paths/glob patterns were implemented (#266). We can use FileList and ExpressibleByStringLiteral to expose public, private and project headers as either a String or Array of Strings. The original FileList used for sources was not reused as it contained compilerFlags which is specific to source files. A new FileList containing only paths/glob patterns was created for use with headers. This way FileList can be potentially reused elsewhere.

Implementation 👩‍💻👨‍💻

  • Remove FileList type alias from SourceFilesList and references to it
  • Create new FileList without compilerFlags
  • Modify Headers to use the new FileList for public, private and project headers
  • Add tests to GeneratorModelLoaderTests to verify newly supported cases work
  • Create ios_app_with_headers fixture to support new array of strings case for headers
  • Write test that verifies ios_app_with_headers fixture targets build

Test Plan

  • Run tuist generate inside the ios_app_with_headers fixture
  • Verify that the Framework1 target has the correct header files specified in the manifest

@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

Copy link
Contributor

@pepicrft pepicrft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this @adamkhazi. I have only one comment.
What about adding a new fixture project that contains targets with headers initialized with a string and with an array of strings? Since this is a user-facing feature, having the test there will help us detect breaking changes in the future.

Moreover, don't forget to update the documentation under the /docs directory to reflect these changes.

Other than that, the code looks great.

@adamkhazi adamkhazi force-pushed the feature/multiple-header-paths branch from 1844919 to e70706a Compare July 25, 2019 08:30
@codecov
Copy link

codecov bot commented Jul 25, 2019

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #459      +/-   ##
==========================================
+ Coverage   92.11%   92.15%   +0.03%     
==========================================
  Files         319      320       +1     
  Lines       16567    16650      +83     
==========================================
+ Hits        15261    15344      +83     
  Misses       1306     1306
Impacted Files Coverage Δ
Sources/ProjectDescription/SourceFilesList.swift 78.94% <ø> (ø) ⬆️
Sources/ProjectDescription/Target.swift 100% <ø> (ø) ⬆️
...nerator/TestData/ProjectDescription+TestData.swift 100% <ø> (ø) ⬆️
...rces/TuistKit/Generator/GeneratorModelLoader.swift 86.19% <100%> (+0.33%) ⬆️
Sources/ProjectDescription/FileList.swift 100% <100%> (ø)
Tests/ProjectDescriptionTests/TargetTests.swift 100% <100%> (ø) ⬆️
Tests/ProjectDescriptionTests/HeadersTests.swift 100% <100%> (ø) ⬆️
...KitTests/Generator/GeneratorModelLoaderTests.swift 97.93% <100%> (+0.24%) ⬆️
Sources/ProjectDescription/Headers.swift 100% <100%> (ø) ⬆️

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 c148243...a25a362. Read the comment docs.

@adamkhazi adamkhazi merged commit f84fe72 into tuist:master Jul 25, 2019
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.

Support multiple header paths in the project manifest
3 participants