Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add watchOS 7 and tvOS 14 check for logger #4

Merged
merged 1 commit into from
Oct 3, 2022

Conversation

MartinP7r
Copy link
Contributor

Hi there!

I just realized you added this check only for some platforms supported in your Package.swift.

@MartinP7r
Copy link
Contributor Author

Hello @jayhickey, could you have a look at this PR if you have time?

Copy link
Owner

@jayhickey jayhickey 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 the PR @MartinP7r and sorry for the delay! Great catch here, just one additional ask to sync these versions with the Package.swift

@@ -98,7 +98,7 @@ public final class SyncEngine<Model: CloudKitCodable> {

self.logHandler =
logHandler ?? { string, level in
if #available(iOS 14.0, macOS 11.0, *) {
if #available(iOS 14.0, macOS 11.0, watchOS 7.0, tvOS 14.0, *) {
Copy link
Owner

Choose a reason for hiding this comment

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

Mind updating this to match the Package.swift? I think we can do iOS 14.0, macOS 11.0, watchOS 6.0, tvOS 13.0 in both places

Copy link
Contributor Author

@MartinP7r MartinP7r Oct 3, 2022

Choose a reason for hiding this comment

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

Hi @jayhickey

Logger is only available from watchOS 7.0 and tvOS 14.0 respectively. (reference)

It won't compile otherwise. 🙏🏼

@jayhickey jayhickey merged commit 0ef2b82 into jayhickey:main Oct 3, 2022
@jayhickey
Copy link
Owner

Thanks!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants