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

Add support for watchOS apps #623

Merged
merged 17 commits into from Nov 6, 2019

Conversation

@kwridan
Copy link
Contributor

kwridan commented Nov 1, 2019

Short description πŸ“

Adds support for watchOS apps.

Solution πŸ“¦

Add support for watchOS / watchOS Extension products as well as appropriate dependency rules.

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

  • Add product types
  • Add fixture & acceptance test
  • Update graph / link rules
  • Add embed watch build phase
  • Add watch build settings
  • Update changelog

Test Plan πŸ› 

  • Run tuist generate within fixtures/ios_app_with_watchapp2
  • Verify the generated project contains a Watch App & Watch App Extension
  • Compare against a manually created project with a Watch App

Notes

For follow up PRs:

  • Update schemes to include a runnable watch apps (requires using RemoteRunnable launch actions)
  • Add support for watch app containers (aka standalone watch apps)
@kwridan kwridan requested a review from tuist/core Nov 1, 2019
kwridan added 2 commits Nov 1, 2019
fileElements: ProjectFileElements,
pbxproj: PBXProj) throws {
let targetDependencies = graph.targetDependencies(path: path, name: target.name)
let watchApps = targetDependencies.filter { $0.target.product == .watch2App }

This comment has been minimized.

Copy link
@pepibumur

pepibumur Nov 2, 2019

Contributor

Interesting, I would have expected the watch app to have the iOS as a dependency, like we do with tests having the app as a dependency.

This comment has been minimized.

Copy link
@kwridan

kwridan Nov 2, 2019

Author Contributor

Indeed this is an interesting one, using dependency paradigms for those can be thought of going either way depending how you look at it.

I opted to keep it consistent with how extensions are currently defined, they are fairly similar conceptually where they are companion products bundled with the main application - as such the application isn't complete without them.

 Target(name: "App",
        platform: .iOS,
        product: .app,
        bundleId: "io.tuist.App",
        infoPlist: "Support/App-Info.plist",
        sources: ["Sources/**"],
        dependencies: [

            // Extensions + Additional Bundled Products
            .target(name: "TodayExtension"),
            .target(name: "NotificationServiceExtension"),
            .target(name: "WatchApp"),
        ]),

For tests, its slightly different as often the test will actually depend on the app (@testable import App) additionally the test targets need to reference the host / target application either in their build settings or in their target attributes thus the dependency was specified in that direction.

This comment has been minimized.

Copy link
@pepibumur

pepibumur Nov 4, 2019

Contributor

I opted to keep it consistent with how extensions are currently defined, they are fairly similar conceptually where they are companion products bundled with the main application - as such the application isn't complete without them.

That's a fair reason πŸ‘ and the example that you showed is very clear.

kwridan added 2 commits Nov 2, 2019
@kwridan kwridan requested a review from tuist/core Nov 4, 2019
Copy link
Contributor

pepibumur left a comment

The PR looks great to me. Dumping a thought here. One of the issues that I ran in the past with watch apps and extensions was not using the right bundle identifier for the extension. If I'm not wrong, the bundle id of the extension has to be the same of the app suffixed by .watchextension. What do you think about adding a check for that? It took me a long time to figure out what was the issue and considering we know the convention, we can codify it into the Tuist linters.

Also, don't forget to update the documentation.

@kwridan

This comment has been minimized.

Copy link
Contributor Author

kwridan commented Nov 4, 2019

If I'm not wrong, the bundle id of the extension has to be the same of the app suffixed by .watchextension. What do you think about adding a check for that? It took me a long time to figure out what was the issue and considering we know the convention, we can codify it into the Tuist linters.

πŸ‘ good idea - will update

@kwridan kwridan merged commit 7bb8ad7 into tuist:master Nov 6, 2019
5 of 7 checks passed
5 of 7 checks passed
Swiftlint
Details
Rubocop Rubocop
Details
Changelog Changelog
Details
Header rules No header rules processed
Details
Pages changed 44 new files uploaded
Details
Redirect rules No redirect rules processed
Details
Mixed content No mixed content detected
Details
@kwridan kwridan deleted the bloomberg:feature/watchkitapp branch Nov 6, 2019
@kwridan kwridan mentioned this pull request Nov 6, 2019
1 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.