Skip to content

feat: add the App Clip's URL to the Run action #185

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.

Sign up for GitHub

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

Jump to bottom
Merged
fortmarek merged 3 commits into from
Jun 30, 2025
Merged

feat: add the App Clip's URL to the Run action #185

fortmarek merged 3 commits into from
Jun 30, 2025

Conversation

@ns-vasilev
Copy link
Contributor

@ns-vasilev ns-vasilev commented May 2, 2025

No description provided.

@ns-vasilev ns-vasilev force-pushed the feature/app-clips branch from f987e8e to 6bb7627 Compare May 5, 2025 09:13
@ns-vasilev
Copy link
Contributor Author

ns-vasilev commented Jun 5, 2025 •
edited
Loading

@pepicrft @fortmarek Hi folks! 👋

Would appreciate it if you could take a look at this PR when you have a chance
I’m currently unable to add App Clips support as described in issue #7036.

Any feedback or a merge would be greatly appreciated 🙏

fortmarek
fortmarek reviewed Jun 5, 2025
View reviewed changes
Copy link
Member

@fortmarek fortmarek left a comment

Choose a reason for hiding this comment

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

What do you think of using URL and only converting the type to String during generation?

Sources/XcodeGraph/Models/RunAction.swift Outdated
public let metalOptions: MetalOptions?
public let expandVariableFromTarget: TargetReference?
public let launchStyle: LaunchStyle
public let appClipInvocationURLString: String?
Copy link
Member

@fortmarek fortmarek Jun 5, 2025

Choose a reason for hiding this comment

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

Suggested change
public let appClipInvocationURLString: String?
public let appClipInvocationURL: URL?

@fortmarek
Copy link
Member

fortmarek commented Jun 5, 2025

Would appreciate it if you could take a look at this PR when you have a chance

sorry, this has fallen through the cracks. Feel free to ping us sooner if we forget about something 🙏

@fortmarek
Copy link
Member

fortmarek commented Jun 5, 2025

@ns-vasilev what do you think about preparing a PR in tuist/tuist that uses the changes from here? You can reference a fork in the Package.swift. It usually helps to make better decision about the XcodeGraph API.

@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Jun 8, 2025
@ns-vasilev ns-vasilev force-pushed the feature/app-clips branch from 539b60d to 5d08eea Compare June 8, 2025 08:10
@ns-vasilev ns-vasilev force-pushed the feature/app-clips branch from 5d08eea to de8fd78 Compare June 8, 2025 08:15
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Jun 8, 2025
@ns-vasilev ns-vasilev mentioned this pull request Jun 10, 2025
Merged
6 tasks
@ns-vasilev
Copy link
Contributor Author

ns-vasilev commented Jun 10, 2025

@fortmarek, changed the type from String to URL and opened a PR in Tuist with these changes

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 11, 2025
@ns-vasilev
Copy link
Contributor Author

ns-vasilev commented Jun 28, 2025

@fortmarek @pepicrft could you please merge this? Then I can proceed with the integration of AppClips into Tuist.

fortmarek
fortmarek approved these changes Jun 30, 2025
View reviewed changes
@fortmarek
Copy link
Member

fortmarek commented Jun 30, 2025

Thanks and sorry for the longer review cycle here!

@fortmarek fortmarek merged commit f38a8dc into tuist:main Jun 30, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pepicrft pepicrft pepicrft approved these changes

@fortmarek fortmarek fortmarek approved these changes

Assignees

No one assigned

Labels

lgtm This PR has been approved by a maintainer size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ns-vasilev @fortmarek @pepicrft