-
Notifications
You must be signed in to change notification settings - Fork 25
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
Refreshed and face-lifted README. Tested with Chaos Calmer. Worked wit... #17
base: master
Are you sure you want to change the base?
Conversation
|
||
You're going to use menuconfig to set a few settings for the build. | ||
|
||
(not as root!) |
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 this really need to be specified? Is there any reason it won't work as root? If it absolutely must be here, then IMO it shouldn't be indented like code
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 it need to be? No, of course not. At the end of the day, though, this is a fairly substantial vector for people who haven't, or have only rarely, run make. It's quite plausible that they might get permission denied errors and try to sudo. If they do, they'll be in a pretty bad permissions situation. So why not gently remind them in one line. There's no prompt before it; they aren't going to mistake it for code (similar to the menu selection description). It's fine.
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.
ehh, I feel like if it becomes an issue we could just put a big bold thing at the top of the doc that says HEY IDIOTS: DONT DO THIS SHIT AS ROOT
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 know you, so I'll give you the benefit of the doubt and assume
you're kidding.
On Tue, Aug 12, 2014 at 9:10 PM, Finn notifications@github.com wrote:
In README.md:
+#Building
+
+You'll need to know the chipset of the device for which you intend to build your OpenWrt with meshbox.
+
+You're going to use menuconfig to set a few settings for the build.
+
- (not as root!)
ehh, I feel like if it becomes an issue we could just put a big bold thing
at the top of the doc that says HEY IDIOTS: DONT DO THIS SHIT AS ROOT—
Reply to this email directly or view it on GitHub
https://github.com/SeattleMeshnet/meshbox/pull/17/files#r16153161.
Justin Holmes
Chief Chocobo Breeder, slashRoot
slashRoot: Coffee House and Tech Dojo
New Paltz, NY 12561
845.633.8330
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.
Well, kidding about the particular phrasing, but not the overall concepts
Might be worth telling people they should read http://wiki.openwrt.org/doc/howto/build first since it's the authoritative dox for running buildroot. It includes the non-root and troubleshooting sections. |
Agreed. That's a really great doc. On Tue, Aug 12, 2014 at 9:34 PM, Kurt Snieckus notifications@github.com
Justin Holmes slashRoot: Coffee House and Tech Dojo |
Very cool. I agree that a detailed guide like this should be easily accessible. But I think another point to consider is that instructions this verbose can also intimidate people. So how about leaving the old doc as a sort of "Quick Install" on the top, and then a "Detailed Install" with the verbose instructions right below it? |
Alright... I'd even go for the wiki link above in below Building OpenWrtRead about compiling OpenWrt here: http://wiki.openwrt.org/doc/howto/build |
This is the pull request I said I'd make earlier today.