-
Notifications
You must be signed in to change notification settings - Fork 65
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
Update for work state to 2023.08.01 #122
base: master
Are you sure you want to change the base?
Update for work state to 2023.08.01 #122
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.
Mostly looks fine. Have a few questions but I don't see anything here that is functionally problematic.
I'm not clear on why view_log and wda_wrapper have been moved into subdirectories, but I also have no issue if that is done.
The documentation changes are quite messy, and a lot of that documentation needs to be cleaned up / revamped. It was though in bad shape to begin with, so I'm not really concerned.
Are there PRs in other repos associated with this PR?
@@ -1,13 +1,12 @@ | |||
module coordinator.go | |||
|
|||
go 1.12 | |||
go 1.16 |
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 don't see any issue with updating the base requirement to 1.16. I am curious if there is any specific need to do so?
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.
This file was generated on version 16, left it as it is.
Reason - 1.12 is deprecated and more unavailable in Homebrew:
https://formulae.brew.sh/formula/go#default
1.16 now the most wounding available version
@@ -115,7 +115,7 @@ func proc_device_ios_unit( o ProcOptions, uuid string, curIP string) { | |||
return true | |||
} | |||
o.procName = "stf_device_ios" | |||
o.binary = "/usr/local/opt/node@12/bin/node" | |||
o.binary = "/usr/local/opt/node@14/bin/node" |
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.
Does latest STF now require node 14? I set it this way just because it was what was installed on my machine. Really this should be changed to auto-detect which Node is installed and use the latest acceptable version. ( which may not be the latest if STF itself doesn't support the latest. )
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.
node@12 now is more unavailable in Homebrew:
https://formulae.brew.sh/formula/node#default
For this reason, the version is raised to the minimum accessible and check working provider on node@14.
Current used version NodeJS in STF view 17:
https://github.com/DeviceFarmer/stf/blob/master/Dockerfile-debian-x86_64#L29
I think that updating the version of NodeJs is actually better done as part of a separate task. There is a PR about this:
#118
docs/DEPLOYMENT.md
Outdated
# Deployment | ||
|
||
## Relevance | ||
This instruction actual on 2023/08/09 |
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 is this date here for? People can see when the docs were last updated by looking at the commit 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.
Fix it.
docs/DEPLOYMENT.md
Outdated
This instruction actual on 2023/08/09 | ||
|
||
## Tested working configuration | ||
- PC: MacMini 2018 (x86_64) |
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 don't see how the physical hardware matters. It would not change anything.
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.
remove it
docs/DEPLOYMENT.md
Outdated
|
||
## Tested working configuration | ||
- PC: MacMini 2018 (x86_64) | ||
- OS: MacOS Ventura 13.5 |
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.
No part of MacOS is used. MacOS in the sense of the repo is only used as a Linux environment really that has usbmuxd running. In MacOS case it is the Apple usbmuxd implementation, so this could make some small difference, but I don't think is worth mentioning.
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.
The MacOS version can affect the available Homebrew packages and the available XCode version.
For this reason, it is indicated
Now libusbmuxd dont compile on MacOS 12 and requires MacOS > 13.0 and new XCode.
Fix info
docs/DEPLOYMENT.md
Outdated
- PC: MacMini 2018 (x86_64) | ||
- OS: MacOS Ventura 13.5 | ||
- IDE: XCode 14.3.1 | ||
- Mobile devices: iPhone and iPad different years with iOS versions: 14.1, 14.2, 14.6, 14.7, 15.0, 15.4.1, 16.6 |
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.
A very wide set of device versions beyond this has been tested. It long since became irrelevant to list which versions have been tested. If versions don't work they should just have an open issue.
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.
remove it
1. Click `+` in the lower left corner | ||
1. Select `Apple Development` | ||
1. Install [Homebrew](https://docs.brew.sh/Installation) | ||
1. Install Python 2.7 from MacPorts |
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 don't think I ever said this. Why install Python from MacPorts?
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.
Python 3 is default from MacOS 11:
https://developer.apple.com/documentation/macos-release-notes/macos-catalina-10_15-release-notes#Deprecations
To use Python 2.7, you need to install it separately. Homebrev does not support the installation of Python, so MacPorts remain the most convenient way.
docs/DEPLOYMENT.md
Outdated
1. Download actual profiles for device by Xcode or TestFlight: | ||
https://docs.fastlane.tools/actions/sigh/ | ||
|
||
## 4. Prepare dependenses on build machine |
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.
MIsspelling. It is spelled "dependencies"
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.
Thanks, fix it
docs/DEPLOYMENT.md
Outdated
|
||
In Homebrew no formulae for it: https://github.com/Homebrew/homebrew-core/pull/87059 | ||
|
||
1. Resolve by: https://github.com/libimobiledevice/libimobiledevice/issues/1217 |
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 pile of junk really needed??
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.
Fix it
docs/DEPLOYMENT.md
Outdated
} | ||
``` | ||
|
||
1. Fix [error](https://github.com/DeviceFarmer/stf_ios_support/issues/100) in repos `ios_video_pull` |
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.
This should be addressed by having a mentioned PR for that repo also that gets accepted along with this one.
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.
fix doc
dryark/ios_video_pull#2
Add new PR by comment: dryark/ios_video_pull#2 |
@nanoscopic hi! |
@nanoscopic remind me of PR |
@nanoscopic hello When will it be reviewed and merged? |
@nanoscopic , I am trying to setup iOS provider in Mac mini m2 , Mac Samoa OS. Do I need to take care of anything extra to make it work with arm based Mac system? |
libimobiledevice-glue (new formula) Homebrew/homebrew-core#87059
Homebrew installation of libimobiledevice from HEAD fails with missing package libimobiledevice-glue-1.0 libimobiledevice/libimobiledevice#1217
https://githubhelp.com/devicefarmer/stf_ios_support/issues/37
https://cdmana.com/2021/07/20210721023059534z.html
Failed setup on Mac OS Catalina #100
Failed setup on Mac OS Catalina #100
/bin/sh: ./Scripts/bootstrap.sh: No such file or directory #95
https://github.com/mbilbiesi/stf_ios_support/pull/1/files
https://juejin.cn/post/6985818094794965006#heading-6
Failed setup on Mac OS Catalina #100
/bin/sh: ./Scripts/bootstrap.sh: No such file or directory #95
https://github.com/mbilbiesi/stf_ios_support/pull/1/files
random mismash of issues #24
@koral-- Hi!
I don't see anymore repository: https://github.com/DeviceFarmer/stf_ios_support for create PR(