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.

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

Update podspec script to remove 'S' files from watchOS target #277

Merged
merged 2 commits into from
Feb 24, 2021
Merged

Update podspec script to remove 'S' files from watchOS target #277

merged 2 commits into from
Feb 24, 2021

Conversation

daltonclaybrook
Copy link
Contributor

This PR resolves #276

@swift-server-bot
Copy link

Can one of the admins verify this patch?

9 similar comments
@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@Lukasa
Copy link
Contributor

Lukasa commented Feb 23, 2021

@daltonclaybrook Should this patch also provide the #NO_ASM flag for watchOS, so we can get you all the way to “works fine” for your use-case?

@daltonclaybrook
Copy link
Contributor Author

@Lukasa Good thinking, just added that to the podspec. Let me know if this looks correct to you. In my testing, when I use $(inherited) with GCC_PREPROCESSOR_DEFINITIONS, the Pods project ends up with two instances of that variable. Not sure why this is happening, but I omitted it.
GCC_PREPROCESSOR_DEFINITIONS

Lukasa
Lukasa approved these changes Feb 23, 2021
View reviewed changes
Copy link
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

This LGTM, I think. @glbrntt want to take a look as well?

@Lukasa Lukasa added the semver/patch No public API change. label Feb 23, 2021
@Lukasa
Copy link
Contributor

Lukasa commented Feb 23, 2021

@swift-nio-bot test this please

glbrntt
glbrntt approved these changes Feb 24, 2021
View reviewed changes
Copy link
Contributor

@glbrntt glbrntt 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 to me.

@glbrntt glbrntt merged commit bbb38fb into apple:main Feb 24, 2021
@daltonclaybrook daltonclaybrook deleted the dalton/podspec_without_watchos_assembly_files branch February 24, 2021 15:01
@daltonclaybrook
Copy link
Contributor Author

Can the podspec be republished for 2.10.3, or will it have to wait for another version?

@Lukasa
Copy link
Contributor

Lukasa commented Feb 24, 2021

I don't believe we can publish a pod spec for the same version again. However, I'm open to cutting a new release explicitly to provide the new pod spec. @glbrntt, how do you feel here?

@glbrntt
Copy link
Contributor

glbrntt commented Feb 24, 2021

New release sounds fine to me.

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

@glbrntt glbrntt glbrntt approved these changes

@Lukasa Lukasa Lukasa approved these changes

Assignees
No one assigned
Labels
semver/patch No public API change.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove 'S' files from watchOS target in CocoaPods
4 participants
@daltonclaybrook @swift-server-bot @Lukasa @glbrntt