Skip to content

Add .extensionKitExtension as the new PBXProductType#691

Merged
kwridan merged 4 commits into
tuist:mainfrom
mtj0928:add-extension-kit
Jun 29, 2022
Merged

Add .extensionKitExtension as the new PBXProductType#691
kwridan merged 4 commits into
tuist:mainfrom
mtj0928:add-extension-kit

Conversation

@mtj0928

@mtj0928 mtj0928 commented Jun 25, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

@kwridan kwridan left a comment

Copy link
Copy Markdown
Collaborator

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

kwridan commented Jun 25, 2022

Copy link
Copy Markdown
Collaborator

@all-contributors add @mtj0928 for code

@allcontributors allcontributors Bot mentioned this pull request Jun 25, 2022
Merged
@allcontributors

allcontributors Bot commented Jun 25, 2022

Copy link
Copy Markdown
Contributor

@kwridan

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

@mtj0928

mtj0928 commented Jun 26, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@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

freddi-kit commented Jun 28, 2022

Copy link
Copy Markdown
Contributor

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

@kwridan kwridan left a comment •
edited
Loading

Copy link
Copy Markdown
Collaborator

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

kwridan commented Jun 28, 2022

Copy link
Copy Markdown
Collaborator

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

danieleformichelli commented Jun 28, 2022

Copy link
Copy Markdown
Collaborator

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

@fortmarek

fortmarek commented Jun 28, 2022

Copy link
Copy Markdown
Member

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

kwridan commented Jun 28, 2022

Copy link
Copy Markdown
Collaborator

@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

@kwridan kwridan left a comment

Copy link
Copy Markdown
Collaborator

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

codecov-commenter commented Jun 29, 2022 •
edited
Loading

Copy link
Copy Markdown

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

mtj0928 commented Jun 29, 2022

Copy link
Copy Markdown
Contributor Author

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

@kwridan kwridan left a comment

Copy link
Copy Markdown
Collaborator

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

freddi-kit commented Jun 29, 2022

Copy link
Copy Markdown
Contributor

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

@kwridan

kwridan commented Jun 30, 2022

Copy link
Copy Markdown
Collaborator

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