Skip to content

Add .extensionKitExtension as the new PBXProductType #691

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
kwridan merged 4 commits into from
Jun 29, 2022
Merged

Add .extensionKitExtension as the new PBXProductType #691

kwridan merged 4 commits into from
Jun 29, 2022

Conversation

@mtj0928
Copy link
Contributor

@mtj0928 mtj0928 commented Jun 25, 2022 •
edited
Loading

Resolves: #687

Short description 📝

From Xcode14 beta 1, com.apple.product-type.extensionkit-extension was introduced as the new productType.
The new productType is used for a new app extension like AppIntents Extension.

Solution 📦

Added .extensionKitExtension as the new case of PBXProductType in this PR, to support the new productType.

Implementation 👩‍💻👨‍💻

  • Added .extensionKitExtension as the new case of PBXProductType
  • Added fileExtension of .extensionKitExtension
  • Added a new UnitTest for .extensionKitExtension

kwridan
kwridan reviewed Jun 25, 2022
View reviewed changes
Copy link
Collaborator

@kwridan kwridan 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 contributing this @mtj0928

For consistency other product types (e.g. appExtension), I was thinking perhaps .extensionKitExtension? I appreciate the repeat of "extension" word is a bit odd 😅

@kwridan
Copy link
Collaborator

kwridan commented Jun 25, 2022

@all-contributors add @mtj0928 for code

@allcontributors allcontributors bot mentioned this pull request Jun 25, 2022
Merged
@allcontributors
Copy link
Contributor

allcontributors bot commented Jun 25, 2022

@kwridan

I've put up a pull request to add @mtj0928! 🎉

@mtj0928
Copy link
Contributor Author

mtj0928 commented Jun 26, 2022 •
edited
Loading

@kwridan
Thank you for your comment!!

I actually considered the name at first , but I also felt it was a bit odd.
Would you like me to change the name to extensionKitExtension?

@freddi-kit
Copy link
Contributor

freddi-kit commented Jun 28, 2022

I vote to rename as extensionKitExtension to make naming consistently to others (appExtension, tvExtension)

kwridan
kwridan approved these changes Jun 28, 2022
View reviewed changes
Copy link
Collaborator

@kwridan kwridan left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thinking through it some more, the concern I had was around the possibility of there being extensionKit<SomethingElse> by the structure of the current product ID.

That said, we can start out with .extensionKit which is less odd, should any future products be added that extend this (e.g. .extensionKitHost) we could deprecate .extensionKit and re-introduce the specialised .extensionKitExtension.

So let's proceed with .extensionKit.

@kwridan
Copy link
Collaborator

kwridan commented Jun 28, 2022

cc: @tuist/core - any preference to the naming convention - I could see either option working, consistency vs oddness 😅

@kwridan kwridan requested review from a team, RomainBoulay, andreacipriani, danieleformichelli and fortmarek and removed request for a team June 28, 2022 06:05
@danieleformichelli
Copy link
Collaborator

danieleformichelli commented Jun 28, 2022

Slightly in favor of extensionKitExtension to make clear it's an extension, but no strong opinion

@fortmarek
Copy link
Member

fortmarek commented Jun 28, 2022

I'm ok with merging this but I am also leaning to use the more consistent extensionKitExtension. While it does sound weird, I prefer it over having to deal with it down the road if we need to differentiate between different types for extensionKit.

@kwridan
Copy link
Collaborator

kwridan commented Jun 28, 2022

@mtj0928 can we trouble you to rename to .extensionKitExtension?

The points raised by @fortmarek are valid, while the name is odd, it is consistent with other product types and the underlying Xcode types.

kwridan
kwridan requested changes Jun 28, 2022
View reviewed changes
Copy link
Collaborator

@kwridan kwridan left a comment

Choose a reason for hiding this comment

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

For visibility marking as request changes. thanks again!

@codecov-commenter
Copy link

codecov-commenter commented Jun 29, 2022 •
edited
Loading

Codecov Report

Merging #691 (ac41122) into main (4dccbc0) will decrease coverage by 0.00%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##             main     #691      +/-   ##
==========================================
- Coverage   84.93%   84.92%   -0.01%     
==========================================
  Files         157      157              
  Lines        9098     9101       +3     
==========================================
+ Hits         7727     7729       +2     
- Misses       1371     1372       +1
Impacted Files Coverage Δ
...ces/XcodeProj/Objects/Targets/PBXProductType.swift 54.28% <50.00%> (-1.60%) ⬇️
...rojTests/Objects/Targets/PBXProductTypeTests.swift 100.00% <100.00%> (ø)

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 4dccbc0...ac41122. Read the comment docs.

@mtj0928
Copy link
Contributor Author

mtj0928 commented Jun 29, 2022

Thank you for your discussion, I renamed the new case and pushed the changes😄

kwridan
kwridan approved these changes Jun 29, 2022
View reviewed changes
Copy link
Collaborator

@kwridan kwridan left a comment

Choose a reason for hiding this comment

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

Thanks @mtj0928

@kwridan kwridan changed the title Add .extensionKit as the new PBXProductType Add .extensionKitExtension as the new PBXProductType Jun 29, 2022
@kwridan kwridan merged commit 2fe19ca into tuist:main Jun 29, 2022
@freddi-kit
Copy link
Contributor

freddi-kit commented Jun 29, 2022

Hello, @kwridan, when is it released as new version?

@kwridan
Copy link
Collaborator

kwridan commented Jun 30, 2022

Let's wait till #694 is merged then we can do a release with a few of the Xcode 14 related items within it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@danieleformichelli danieleformichelli danieleformichelli approved these changes

@fortmarek fortmarek fortmarek approved these changes

@kwridan kwridan kwridan approved these changes

@RomainBoulay RomainBoulay Awaiting requested review from RomainBoulay

@andreacipriani andreacipriani Awaiting requested review from andreacipriani

+1 more reviewer

@freddi-kit freddi-kit freddi-kit approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add support for ExtensionKit extension product types (new in Xcode 14)

6 participants

@mtj0928 @kwridan @freddi-kit @danieleformichelli @fortmarek @codecov-commenter