-
Notifications
You must be signed in to change notification settings - Fork 435
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
Add macos support #184 #217
base: master
Are you sure you want to change the base?
Conversation
SwiftLint found issuesWarnings
Generated by 🚫 Danger |
Implement some basic method on current device (system name and version, model name)
manually removed the double and old mac devices |
Source/Device.generated.swift
Outdated
/// Device is an [iMac Pro](https://support.apple.com/kb/SP771) | ||
/// | ||
/// ![Image](https://support.apple.com/library/APPLE/APPLECARE_ALLGEOS/SP771/SP771-imac-pro-2017.png) | ||
case iMacPro |
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.
I suggest changing this to iMaxPro2017
to prevent having to introduce a breaking change at a later date.
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.
✅
Next to the little thing a very nice addition! |
Thanks for the review. I updated manually the iMacPro name I make different output formats |
@@ -286,6 +288,239 @@ public enum Device { | |||
/// | |||
/// ![Image](https://support.apple.com/library/APPLE/APPLECARE_ALLGEOS/SP808/sp808-apple-watch-series-5_2x.png) | |||
case appleWatchSeries5_44mm | |||
#elseif os(macOS) |
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.
Are you sure os(macOS)
is true when running on a Catalyst target?
If not, we can use:
#elseif os(macOS) || targetEnvironment(macCatalyst)
to support both Catalyst and native macOS apps.
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.
os(macOS)
return false on Catalyst. os(iOS)
return true.
catalyst is an iOS device so the code compiled will be the one create before with #if os(iOS)
I make this PR to see if catalyst must be managed well by DeviceKit, and cut my work into two features. First macOS , then macCatalyst
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.
Okay that fine then.
New macbook pro 16 inch 2019https://support.apple.com/kb/SP809 https://www.apple.com/macbook-pro-16/ but no model identifier yet! https://support.apple.com/en-us/HT201300 |
Source/Device.swift.gyb
Outdated
@@ -331,6 +672,8 @@ public enum Device { | |||
case .simulator(let model): return model.diagonal | |||
case .unknown: return -1 | |||
} | |||
#elseif os(macOS) | |||
return -1 |
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.
return -1 | |
return -1 |
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.
I will do it to be compliant with the switch, but I think there is some issues with formatting
because now #if #elseif do not shift the code anymore
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.
formatting done ✅
Source/Device.swift.gyb
Outdated
"macMini2018", | ||
"Device is a [Mac mini (2018)](https://support.apple.com/kb/SP782)", | ||
"https://support.apple.com/library/content/dam/edam/applecare/images/en_US/macmini/mac-mini-2018-space-gray.jpg", | ||
["Macmini8,1"], 0,(), "Mac mini (2018)", -1, False, False, False, False, False, False, False, False, False, 0, False, 0), |
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.
["Macmini8,1"], 0,(), "Mac mini (2018)", -1, False, False, False, False, False, False, False, False, False, 0, False, 0), | |
["Macmini8,1"], 0, (), "Mac mini (2018)", -1, False, False, False, False, False, False, False, False, False, 0, False, 0), |
Applies to all other devices as well...
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.
add missing space in template done ✅
@phimage Sorry for late reply and inactivity. I found some formatting inconsistencies. Please fix those.
|
Add missing space before `return -1`
case macPro2019 With also some missing macPro that apple add to the its support page |
Hey, any updates on this? Looks pretty useful. |
@phimage @Zandor300 anything that blocks merge? 😄 |
Hello, it is a really useful PR do you plan to merge it soon? Thank you! |
Would also love to see this merged. |
Why wasn't this merged...? No more discussion on it, and this is just stagnant.. very important feature for DeviceKit imo. |
@Zandor300 would love to see this getting merged as well! 🙏 |
Hi, my Pull Request is very old, a lot of change happens on DeviceKit (see conflicts and new features) If someone want macOS I think, first is to make a new Pull Request (and maybe be inspired by this one) |
I do want to take a look at this still at some point, just need to find the time... |
Codecov Report
@@ Coverage Diff @@
## master #217 +/- ##
=============================
=============================
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Bumping this. It would be good to get support in for iOS apps running on Apple Silicon. I was able to get some basic functionality up using the ProcessInfo.processInfo.isiOSAppOnMac property and checking the |
FYI, this spinoff project is designed to work on all Apple platforms including macOS and we'd love to have your contributions and suggestions: https://www.github.com/kudit/Device |
The commit allow to compile with SwiftPM and if necessary I can make a PR just for this one
The second one add models dumped from apple web site
There is some double when using "identifier" so I put the PR as WIP
I do not know what to do. Merge into one. Find a way to make some alias with template generations
My very dirty dump script is available here https://github.com/phimage/MacModelDump