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 ListItem to podspec #2048

Merged
merged 4 commits into from
Jun 21, 2024
Merged

Conversation

lyzhan7
Copy link
Contributor

@lyzhan7 lyzhan7 commented Jun 20, 2024

Platforms Impacted

  • iOS
  • visionOS
  • macOS

Description of changes

Continuation of #1939

  • There was a suggestion in that PR to create a separate subspec for both TVC/ListItem to depend on rather than having ListItem depend on all of TVC - followed up on that, and also thought it would be cleaner for the shared tokens/enums that both TVC/ListItem use to be in its own folder, but happy to hear thoughts/go back to only the podspec changes
  • ListItem depended on TVC's tokens, as well as several enums. There were a couple enums used that weren't defined in the TableViewCellTokenSet file - I saw there were a couple other enums defined in TableViewCellTokenSet, so I just moved all the shared ones into that file - but maybe would make more sense to put all the enums in its own file

Binary change

Not adding/removing any source code, so I don't think this PR should impact binary size - but if there's a reason to run this let me know
(how is our binary size impacted -- see https://github.com/microsoft/fluentui-apple/wiki/Size-Comparison)

Verification

pod lib lint passes. Unfortunately each run of this seemed like it took about 20 minutes. If there's a faster way to validate podspec changes in fluentui-apple let me know!

Microsoft Reviewers: Open in CodeFlow

@lyzhan7 lyzhan7 marked this pull request as ready for review June 21, 2024 01:53
@lyzhan7 lyzhan7 requested review from a team as code owners June 21, 2024 01:53
Copy link
Contributor

@mischreiber mischreiber left a comment

Choose a reason for hiding this comment

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

Looks good overall! I'm debating about the name TableViewListShared, but I also don't have a better idea.

@lyzhan7 lyzhan7 merged commit 2d1071a into microsoft:main Jun 21, 2024
7 checks passed
@lyzhan7 lyzhan7 deleted the user/lyzhan/listitem-podspec branch June 22, 2024 01:59
@mischreiber mischreiber mentioned this pull request Jun 24, 2024
3 tasks
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.

5 participants