Skip to content

Improve handling of "extra" files #1771

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

Merged
merged 1 commit into from
Sep 7, 2018

Conversation

neonichu
Copy link
Contributor

@neonichu neonichu commented Aug 30, 2018

In #1507, we started adding "extra" files to the generated Xcode
project. This makes some enhancements to that feature:

  • Introduce a new CLI flag --skip-extra-files to go back to the pre-4.2 behaviour (best behaviour) of not including these files at all
  • Do not add a file reference for "Package.resolved" since this shouldn't be edited by hand and most people also never have to read it
  • Move the "extra" file references after the products group. This may be debatable, but I think it is more clean to have all file references related to the package manifest grouped together at the top instead of having these interspersed throughout the main group

In swiftlang#1507, we started adding "extra" files to the generated Xcode
project. This makes some enhancements to that feature:

- Introduce a new flag `--skip-extra-files` to go back to the pre-4.2
behaviour (best behaviour) of not including these files at all
- Do not add a file reference for "Package.resolved" since this
shouldn't be edited by hand and most people also never have to read it
- Move the "extra" file references after the products group. This may be
debatable, but I think it more clean to have all file references related
to the package manifest grouped together at the top instead of having
these interspersed throughout the main group.
@neonichu neonichu requested a review from aciidgh August 30, 2018 00:11
to: {
$0.xcodeprojOptions.useLegacySchemeGenerator = $1 ?? false
$0.xcodeprojOptions.enableAutogeneration = $2 ?? false
$0.xcodeprojOptions.addExtraFiles = !($3 ?? false)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This may look weird this way, but I thought it would be cleaner to have the option be positive ("add") vs. negative ("skip").

@@ -231,6 +231,9 @@ class GenerateXcodeprojTests: XCTestCase {
let project = try Xcodeproj.generate(projectName: projectName, xcodeprojPath: outpath, graph: graph, options: XcodeprojOptions(), diagnostics: diagnostics)

XCTAssertTrue(project.mainGroup.subitems.contains { $0.path == "a.txt" })

let projectWithoutExtraFiles = try Xcodeproj.generate(projectName: projectName, xcodeprojPath: outpath, graph: graph, options: XcodeprojOptions(addExtraFiles: false), diagnostics: diagnostics)
XCTAssertFalse(projectWithoutExtraFiles.mainGroup.subitems.contains { $0.path == "a.txt" })
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we should create an extra test case for this? Seems easier to share the setup code, though.

@neonichu
Copy link
Contributor Author

@swift-ci please smoke test

@aciidgh aciidgh merged commit 4362e00 into swiftlang:master Sep 7, 2018
@neonichu neonichu deleted the projgen-extra-files branch September 7, 2018 18:02
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.

2 participants