-
Notifications
You must be signed in to change notification settings - Fork 21
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
Added tvOS support #78
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks! Can you clean up the formatting issue?
Is it possible to add a CI job that at least runs our Clippy checks on the tvOS target? It would be nice if this didn't regress quietly by mistake. |
I think it's part of why this PR is marked a draft, but just to be explicit it would also be great if you could tidy up the commit history when you add the CI coverage. Thank you! |
@complexspaces I looked into adding a clippy ci job for tvOS
Also I looked into running just Check which is not as good as Clippy but at least something. |
80cd835
to
e9dc1b9
Compare
@ErikEverson Thanks for taking an initial look at that! I checked out this branch locally, and I think that I was able to get Clippy to work on a recent nightly. Could you see if this is reproducible for you as well? I'm on an M1 macOS 14 system. These are my versions:
Running
If |
@complexspaces Awesome I will see if I get the same on my side. I was putting |
Thanks! Commit history is looking better. A couple suggestions:
So the end state would be something like:
|
Updated security-framework to 2.10 Added cfg target os for tvOS
@cpu @complexspaces I got clippy to work locally and on CI. One quick question should I run clippy against all tvOS targets or just one like iOS currently is?
iOS targets:
|
I don't feel strongly. Matching the existing iOS target seems sensible to me. |
I also don't think this matters too much, so let's just target the one like iOS does. We don't have any architecture or environment specific code in |
e9dc1b9
to
804161c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Looks good to me, I only had some nits about the CI task to suggest.
804161c
to
6fa3383
Compare
I will start preparing a point release 🚀 |
This was included in v/0.3.1. |
Added support to build tvOS targets so that consumers of rustls-platform-verifier can use it with tvOS devices
Target support added for:
Motivation:
How was this tested: