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

fix CBManager crash on macOS <10.15 #337

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Conversation

oletf
Copy link

@oletf oletf commented Sep 7, 2023

my current fix for #335

@oletf
Copy link
Author

oletf commented Sep 12, 2023

actually wondering how format got a fail on a file which is not part of the commit :D

@qdot
Copy link
Contributor

qdot commented Sep 12, 2023

I'm really bad about updating formatting, so it tends to file through. I need to update our CI to warn, not fail, on formatting.

Don't worry about it, I'll try to get the patch in and formatting updated soon.

@qdot
Copy link
Contributor

qdot commented Sep 16, 2023

Ok, looking at this again...

The one problem I see here is that we aren't actually checking the OS platform we're looking at the version of, so this check will run on both macOS and iOS (I build this library into an iOS application). Do we need to do extra target checks here for knowing if we're running on macOS and iOS? Mainly want to make sure before I merge this, as debugging this on iOS is a pain if anything goes wrong.

@oletf
Copy link
Author

oletf commented Sep 18, 2023

yeah I thought about this but forgot to post a thing about it !
should I change the match to cover both ios edge-cases too then ?

@oletf
Copy link
Author

oletf commented Sep 25, 2023

@qdot I don't have any iOS device to try it on, would it be possible for you to get what a dbg!(os_info::get()) does print on your iOS device ?
I'm not sure of what I'd need to match for to check for iOS vs macOS from os_info crate doc...
Thanks forward

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.

2 participants