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

Adding support for project additional files #314

Merged

Conversation

kwridan
Copy link
Collaborator

@kwridan kwridan commented Mar 31, 2019

Part of #202

Short description πŸ“

Often projects may include additional files or folder references that don't need to be part of any build phase (e.g. design references, documentation etc...)

Solution πŸ“¦

Workspace.swift manifest now supports additionalFiles, the same concept can be added to the Project.swift manifest with the same rules.

  • .glob(pattern: "Documentation/**"): glob pattern to one or more files
  • .folderReference(path: "Designs"): single path to a directory that is added as a folder reference

Example:

let project = Project(name: "App",
                      targets: [
                           // ...
                      ],
                      additionalFiles: [
                         "Dangerfile.swift"
                         "Documentation/**",
                         .folderReference(path: "Designs")
                     ])

Screen Shot 2019-03-31 at 8 29 54 PM

Implementation πŸ‘©β€πŸ’»πŸ‘¨β€πŸ’»

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

  • Update Project manifest to include additionalFiles
  • Update models to translate globs to paths
  • Update ProjectFileElements to include additionalFiles
  • Include fixture
  • Update documentation
  • Update changelog

Test Plan βš’

  • Run tuist generate within fixtures/ios_app_with_custom_workspace/App
  • Open the generated workspace
  • Verify Dangerfile.swift is included in the project but isn't part of the resource phase
  • Verify Documentation files are included (with the appropriate group structure that reflects the file system) and that the files aren't copied to resources
  • Verify Server folder reference is included

@tuistbot
Copy link
Contributor

tuistbot commented Mar 31, 2019

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 Mar 31, 2019

Codecov Report

Merging #314 into master will increase coverage by 0.14%.
The diff coverage is 97.77%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #314      +/-   ##
==========================================
+ Coverage   89.32%   89.47%   +0.14%     
==========================================
  Files         278      280       +2     
  Lines       10616    10707      +91     
==========================================
+ Hits         9483     9580      +97     
+ Misses       1133     1127       -6
Impacted Files Coverage Ξ”
...rces/TuistKit/Generator/GeneratorModelLoader.swift 90.82% <100%> (+0.62%) ⬆️
...atorTests/Generator/ProjectFileElementsTests.swift 99.68% <100%> (+0.01%) ⬆️
Sources/ProjectDescription/Workspace.swift 100% <100%> (+12.9%) ⬆️
Sources/TuistGenerator/Generator.swift 78.94% <100%> (ΓΈ) ⬆️
...erator/Generator/WorkspaceStructureGenerator.swift 77.27% <100%> (ΓΈ) ⬆️
...tKitTests/Generator/GeneratorModelLoaderTest.swift 98.36% <100%> (+0.17%) ⬆️
...atorTests/Models/TestData/Workspace+TestData.swift 100% <100%> (ΓΈ) ⬆️
...nerator/TestData/ProjectDescription+TestData.swift 100% <100%> (ΓΈ) ⬆️
...ratorTests/Generator/WorkspaceGeneratorTests.swift 93.69% <100%> (ΓΈ) ⬆️
Sources/TuistGenerator/Models/Project.swift 58% <100%> (+0.85%) ⬆️
... and 10 more

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 2726304...b640532. Read the comment docs.

case targets
case settings
}
public let additionalFiles: [WorkspaceElement]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this still a workspace element?

Copy link
Contributor

Choose a reason for hiding this comment

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

If we'd like to reuse the same type I'd change the name.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

agreed - was looking for better names:

  • Reference, Element: too broad?
  • FileReference, FileElement: Folder references aren't files :/

Will think some more on suitable alternatives, if you had any please do share :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd stick to the Xcode project's convention, FileElement

Copy link
Collaborator

@ollieatkinson ollieatkinson left a comment

Choose a reason for hiding this comment

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

πŸ” Changes look good to me! It's really nice to see that the work from implementing a better Workspace.swift is coming in handy!

@@ -0,0 +1,3 @@
import json
Copy link
Contributor

Choose a reason for hiding this comment

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

πŸ€”

Copy link
Collaborator Author

@kwridan kwridan Apr 1, 2019

Choose a reason for hiding this comment

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

heh - can replace this with a different fixture that doesn't look as dubious - the intention was to show case a folder reference to some sever looking code

@kwridan kwridan merged commit 8d7f15f into tuist:master Apr 3, 2019
@kwridan kwridan deleted the feature/202-1-project-additional-files branch April 3, 2019 08:43
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.

None yet

4 participants