Skip to content
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

First couple of proofreading pages completed. #12

Merged
merged 2 commits into from
Dec 13, 2021
Merged

First couple of proofreading pages completed. #12

merged 2 commits into from
Dec 13, 2021

Conversation

BuckeyeCoder
Copy link
Contributor

BTW, I'm Geek Stocks in case you're wondering WTF? (That's "WHO the fuck?") lol

I'm not sure when I'll have time again to proofread so I'm submitting these now. Mostly minor stuff; one broken link. I do intend to get to all of the site as I have time.

One important note for you to consider though. The demo at the end of Chapter 1 does not contain an os.Exit(0) line. When the student clicks the window frame's X to exit the app, the window goes away but the process still hangs in the terminal.

Now, you may know that already and it may be intentional for all I know. If it is, I agree with your call, because I'm pretty sure the added complexity to get the os.Exit(0) line into the right place would defeat the goal of keeping this chapter SIMPLE.

BUT, if that IS your reason then my opinion is that you need to do an "information" or "warning" callout at the end of the chapter. The hung process looks bad and makes the project look bad for producing a "failed" demo. Just a sentence or two telling the student that "its intentional" and just use ctrl-c to exit the process for now. And that you'll fix this issue in Chapter X (I haven't looked but it does get fixed.)

Something to think about. I didn't propose anything because I didn't want to presuppose your intentions.

Removed superflous commas (when in doubt, leave it out).
Removed the Oxford Comma, mostly (lol).
Minor grammar to help flow better.
Overall this page is in good shape!
Removed superflous dashes like "platform-dependant".
Added missing punctuation.
Fixed 404 broken link intended to list platforms.
@jonegil jonegil merged commit 9aa989e into jonegil:main Dec 13, 2021
@jonegil
Copy link
Owner

jonegil commented Dec 13, 2021

This is good stuff, thank you for taking the time to read through. I can feel the flow improving when reading your versions. Mine were a bit more staccato, yours more natural. Apprecated.

Cheers!

jonegil added a commit that referenced this pull request Feb 20, 2023
First couple of proofreading pages completed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants