-
Notifications
You must be signed in to change notification settings - Fork 0
Update devices list for Android and iOS #4
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
Conversation
| } | ||
|
|
||
| func readDevicesRawDataFromGithubAsLines() ([]string, error) { | ||
| url := "https://raw.githubusercontent.com/pluwen/apple-device-model-list/main/README.md" |
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.
what if they delete it? May be we can just hardcode it for now?
Also parsing a README looks dicey.
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.
Yes, I agree, it's not the most stable approach but the alternative is manually updating it from a Wikipedia page. So we can use this as long as it works and if it ever breaks, we can go back to manual update or find another source 😇
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.
+1 on parsing from README, very sketchy. Specially given the fact that the readme, has and can possibly have more of non device information like Author etc.
Would it be possible to sanitize/test the output before we populate json files?
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.
You can see here that we are picking only devices sections to be processed.
I completely agree that this isn't a perfect approach but isn't this better than manually updating the file?
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.
💯 better than manual. But the only issue I see, which can be resolved with some sanity checks.
- When we paste things manually, we do not alter anything and items were already verified from Wikipedia.
In this case for script, we are fetching it from source, parsing it and storing it. Unless we are certain the script didn't do anything flaky, we are always gonna be worried or do manual checks.
So, I would say, let's go with the script and source and parsing MD, but maybe let's just have 2 small tests to be sure
- Before inserting in json file, we make sure old json items are not decreasing
- There is not item duplication .
If we have these 2 tests, next time we run script and they pass, we won't have to worry about data integrity. What do you think?
|
|
||
| func main() { | ||
| fmt.Println("TODO: Implement this to ease down on the manual process of updating the device data") | ||
| // Fetch devices raw data from https://github.com/pluwen/apple-device-model-list |
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.
is this script run manually or does some action/statup trigger this?
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.
We run it manually whenever we want to update the devices, just like the Android one.
| } | ||
|
|
||
| func readDevicesRawDataFromGithubAsLines() ([]string, error) { | ||
| url := "https://raw.githubusercontent.com/pluwen/apple-device-model-list/main/README.md" |
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.
+1 on parsing from README, very sketchy. Specially given the fact that the readme, has and can possibly have more of non device information like Author etc.
Would it be possible to sanitize/test the output before we populate json files?
Shahroz16
left a comment
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.
Current iOS and Android file looks okay. So I am okay with the PR.
Regarding the script, I think great addition and start and we can improve, but I have few suggestions to confirm the data integrity isn't comprised, which I think are important.
So, if everyone else's concern are addressed, we are good on this.
This PR updates the devices list for Android and iOS.
Android
This one is pretty straightforward, running the android script
iOS
I implemented a script that fetches the Apple devices list from a Github repo listing them. This replaces the Wikipedia page that we were manually pulling data from.
The new data source for iOS devices does have a few differences. I am highlighting differences between old and new Apple devices data sources.
Devices changed
Apple TV (1st generation)→Apple TV 1Apple TV (2nd generation)→Apple TV 2Apple Watch (1st generation)→Apple WatchiPad (3rd/4th generation)→iPad 3/4iPhone SE (1st generation)→iPhone SEiPhone XR→iPhone XʀiPhone SE (2nd generation)→iPhone SE 2iPhone SE (3rd generation)→iPhone SE 3iPhone 5c→iPhone 5C,iPhone XS→iPhone XsDevices added
Apple TV 4K 3Apple Watch SE 2,Apple Watch Series 8,Apple Watch UltraApple Watch Series 9,Apple Watch Ultra 2,Apple Watch Series 10iPad 10,iPad Air (M2),iPad Mini (A17 Pro),iPad Pro (M4)iPhone 15,iPhone 16No devices were removed