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

Bump minimum supported watchOS version #445

Merged

Conversation

sebastian-qm
Copy link
Contributor

Hi!

My app doesn't compile with Xcode 15 (currently in beta). The problem is caused by the opentelemetry-swift package. I got the following errors in Xcode build log:

Build target OpenTelemetryProtocolExporterCommon
The package product 'Logging' requires minimum platform version 5.0 for the watchOS platform, but this target supports 4.0
The package product 'SwiftProtobuf' requires minimum platform version 5.0 for the watchOS platform, but this target supports 4.0

Build target OpenTelemetryProtocolExporterGrpc
The package product 'Logging' requires minimum platform version 5.0 for the watchOS platform, but this target supports 4.0

The package product 'SwiftProtobuf' requires minimum platform version 5.0 for the watchOS platform, but this target supports 4.0
The package product 'GRPC' requires minimum platform version 5.0 for the watchOS platform, but this target supports 4.0

Build target OpenTelemetryProtocolExporterHttp
The package product 'Logging' requires minimum platform version 5.0 for the watchOS platform, but this target supports 4.0
The package product 'SwiftProtobuf' requires minimum platform version 5.0 for the watchOS platform, but this target supports 4.0

I could fix the issue by forking opentelemetry-swift, bumping the watchOS version in your [email protected] and making my app depend on the fork.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 1, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@codecov
Copy link

codecov bot commented Aug 3, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.03% ⚠️

Comparison is base (869dc5a) 66.37% compared to head (df23516) 66.35%.

❗ Current head df23516 differs from pull request most recent head a85429d. Consider uploading reports for the commit a85429d to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #445      +/-   ##
==========================================
- Coverage   66.37%   66.35%   -0.03%     
==========================================
  Files         331      331              
  Lines       14113    14113              
==========================================
- Hits         9367     9364       -3     
- Misses       4746     4749       +3     

see 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bryce-b
Copy link
Member

bryce-b commented Aug 3, 2023

We discussed this change and decided that supporting the Xcode Beta is a concern in an existing package manifest, which could break projects dependent on that manifest using the GA version of Xcode. Can you create a new package manifest for the beta xcode? Name it [email protected]

@bryce-b
Copy link
Member

bryce-b commented Aug 3, 2023

I was looking at the support versions and Apple lists and xcode 15 is suppose to support watchOS 4.0 :
https://developer.apple.com/support/xcode/

@nachoBonafonte
Copy link
Member

Could you also update the other platforms two versions up? We would like to keep versions synchronized

@sebastian-qm
Copy link
Contributor Author

Could you also update the other platforms two versions up? We would like to keep versions synchronized

You mean bumping iOS and tvOS to v13? Sure.

@nachoBonafonte
Copy link
Member

And also macOS, the idea is trying to have all targets with the same base libraries versions. Thanks.

@sebastian-qm
Copy link
Contributor Author

Ah, ok, got it. That would be v10_15 (Catalina), right?

@nachoBonafonte
Copy link
Member

We will also need you to sign the EasyCLA, we cannot merge if not signed

@sebastian-qm
Copy link
Contributor Author

I'm in contact with my manager about the CLA. I think we can work it out next week.

Thank you guys for your quick reviews! 👍

[email protected] Outdated Show resolved Hide resolved
@bryce-b
Copy link
Member

bryce-b commented Aug 10, 2023

@sebastian-qm Any update on getting the CLA signed?

@sebastian-qm
Copy link
Contributor Author

@sebastian-qm Any update on getting the CLA signed?

@bryce-b No, not really, sorry. I was on vacation last week 😎

@sebastian-qm
Copy link
Contributor Author

EasyCLA is signed now.

@sebastian-qm
Copy link
Contributor Author

Hi @vvydier ,
could you please review this PR?

@nachoBonafonte nachoBonafonte merged commit 648dbba into open-telemetry:main Aug 21, 2023
4 checks passed
@vvydier
Copy link
Contributor

vvydier commented Aug 21, 2023

Hi @vvydier , could you please review this PR?

Nacho has merged your PR, Thanks for your contribution!

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.

4 participants