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

A bunch of non-technical changes, mostly minor. #34

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

A bunch of non-technical changes, mostly minor. #34

wants to merge 1 commit into from

Conversation

nobozo
Copy link

@nobozo nobozo commented Jan 16, 2014

I made a bunch of fixes, mostly for consistency. I didn't make any technical changes.
There are still more changes that could be made, but I wanted to see what you think
of what I've done so far before doing them.

@karlseguin
Copy link
Owner

Sorry to be slow on this request. To be honest, I'm pretty hesitate as a lot of this seems subjective and stylistic. For example, I'm not a fan of a comma before "and", and it's my understanding that it's ok either way, so why change it?

@nobozo
Copy link
Author

nobozo commented Feb 1, 2014

On 1/31/2014 5:49 PM, Karl Seguin wrote:

Sorry to be slow on this request. To be honest, I'm pretty hesitate as a
lot of this seems subjective and stylistic. For example, I'm not a fan
of a comma before "and", and it's my understanding that it's ok either
way, so why change it?

It all depends on whether you're trying for a book that's both
technically correct (which I can't comment on) and easy to read.
One way to be easy to read is to be consistent, which you're
not. For example, sometime you use "A, B and C", and sometimes
you use "A, B, and C".

I had said in the pull request that the changes I made were
stylistic. Of course you're free to take them or leave them,
but I believe your book would be of higher quality if you
took them.

Jon

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