-
Notifications
You must be signed in to change notification settings - Fork 266
Make orbic-network the default installer #675
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
248f539 to
1b80745
Compare
1b80745 to
22b1de1
Compare
bmw
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.
idk if anyone wants to do a second review as i haven't reviewed significant PRs on this repo before, but other than my minor inline comment about docs, this lgtm
doc/orbic.md
Outdated
| It's possible that many tutorials out there still refer to some of the old | ||
| installation routines. | ||
|
|
||
| ## The Network Installer |
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.
should we just delete this section? it refers to orbic-network as experimental and i think all information here is now included in installing-from-release.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.
i could've sworn i did that. the intent was to replace them with "The USB installer" docs as the two swapped places but now i can't be arsed to write those docs (also still not sure where to re-add the windows zadig docs)
installer/src/orbic.rs
Outdated
|
|
||
| pub async fn install() -> Result<()> { | ||
| println!( | ||
| "WARNING: The orbic USB installer is likely to go away in a future version of Rayhunter. Consider using ./installer orbic instead." |
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 we actually going to remove it? It does do something useful, which is put the device into adb mode. Very helpful for transferring files as a poweruser.
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 we should not remove the USB installer, for several reasons.
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 did expect us to eventually remove it if we at some point reach feature parity wrt shell and file transfer utils. but point taken.
I would like to encourage people to report bugs with the network installer even if they are currently using the USB Installer. I think we should somehow discourage its use for non power users.
|
I think i addressed all review comments. |
Co-authored-by: Cooper Quintin <[email protected]>
closes #599