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

Improve gitbootcamp.rst #524

Merged
merged 8 commits into from Sep 3, 2019
Merged

Improve gitbootcamp.rst #524

merged 8 commits into from Sep 3, 2019

Conversation

@maggyero
Copy link
Contributor

@maggyero maggyero commented Aug 11, 2019

This PR will apply the following changes to gitbootcamp.rst:

  • fix capitalization in some section titles;
  • use the long form of will consistently;
  • use variables <x> consistently.
Copy link
Member

@aeros aeros left a comment

Thanks for the PR @maggyero!

I would recommend adjusting the title from "Update" to "Improve". Updating would imply that some parts are no longer relevant, and are being updated accordingly. The changes made in this PR would be better described as improvements or fixes.

Overall, I agree with many of the changes made, but I have some suggestions:

gitbootcamp.rst Show resolved Hide resolved
gitbootcamp.rst Outdated Show resolved Hide resolved
gitbootcamp.rst Show resolved Hide resolved
gitbootcamp.rst Outdated Show resolved Hide resolved
gitbootcamp.rst Show resolved Hide resolved
gitbootcamp.rst Show resolved Hide resolved
gitbootcamp.rst Show resolved Hide resolved
gitbootcamp.rst Show resolved Hide resolved
gitbootcamp.rst Show resolved Hide resolved
gitbootcamp.rst Show resolved Hide resolved
@maggyero maggyero changed the title Update gitbootcamp.rst Improve gitbootcamp.rst Aug 13, 2019
maggyero and others added 3 commits Aug 13, 2019
Co-Authored-By: Kyle Stanley <[email protected]>
Co-Authored-By: Kyle Stanley <[email protected]>
@maggyero
Copy link
Contributor Author

@maggyero maggyero commented Aug 13, 2019

@aeros167 Thanks for the review. I have applied some of your suggestions.

gitbootcamp.rst Show resolved Hide resolved
gitbootcamp.rst Outdated Show resolved Hide resolved
gitbootcamp.rst Outdated Show resolved Hide resolved
gitbootcamp.rst Outdated Show resolved Hide resolved
maggyero and others added 3 commits Aug 16, 2019
Co-Authored-By: Emmanuel Arias <[email protected]>
@aeros
aeros approved these changes Aug 23, 2019
@eamanu
eamanu approved these changes Aug 23, 2019
@methane
Copy link
Member

@methane methane commented Aug 24, 2019

  • use variables <x> consistently.

I don't think this is a good idea. Examples helps readers than variables sometime.

For example, "Your Name" looks more clear than "<your name>". It is easy to understand that I should write "Inada Naoki", not "<inada naoki>" or "methane".

@aeros
Copy link
Member

@aeros aeros commented Aug 24, 2019

@methane

I don't think this is a good idea. Examples helps readers than variables sometime.

For example, "Your Name" looks more clear than "". It is easy to understand that I should write "Inada Naoki", not "" or "methane".

In that case, I think we should aim to consistently use one format or the other within the article, as switching between the two could cause some degree of confusion. I'm okay with using quotes instead of tags though.

Another alternative would be to use italics, that format is used commonly in a number of different areas of documentation to tell the reader that they should insert their own value:

git config --global user.name "Your Name"

In that example, the quotation marks have a functional purpose (telling git to treat it as a single argument even with the spaces).

@methane
Copy link
Member

@methane methane commented Aug 29, 2019

In that case, I think we should aim to consistently use one format or the other within the article, as switching between the two could cause some degree of confusion.

I don't think it causes real confusion. Variables are useful when the same word is used several times. But it is not so useful when it is used only once like <name> or <email>.

As I described above, "Your Name" will have less confusion than "<name>" or <name>.

@aeros
Copy link
Member

@aeros aeros commented Aug 29, 2019

@methane:

I don't think it causes real confusion. Variables are useful when the same word is used several times. But it is not so useful when it is used only once like <name> or <email>.

Okay, I see. As an example, do you think it's useful in the following context for <username>? (taken from an existing section in gitbootcamp):

git clone [email protected]:<username>/cpython.git

As I described above, "Your Name" will have less confusion than "<name>" or <name>.

I agree, I just thought that the italics might be useful, such as "Your Name". It helps to make the text stand out to the reader, and indicate they should substitute their own text there. In the specific case of "Your Name" it's fairly clear either way, it just makes it more obvious.

@methane
Copy link
Member

@methane methane commented Aug 30, 2019

Okay, I see. As an example, do you think it's useful in the following context for <username>? (taken from an existing section in gitbootcamp):

Yes. <username> is used above (L48):

   git clone [email protected]:<username>/cpython.git

So <your-username> should be replaced with <username> for consistency.
This is the case consistency is important for readers.

It may be more helpful to add more comment like this:

   # Replace <username> with your github user name.
   git clone [email protected]:<username>/cpython.git
@maggyero
Copy link
Contributor Author

@maggyero maggyero commented Sep 2, 2019

@methane Alright, updated, thanks for the review.

@methane methane merged commit 60b5460 into python:master Sep 3, 2019
6 checks passed
6 checks passed
Header rules No header rules processed
Details
Pages changed 7 new files uploaded
Details
Redirect rules No redirect rules processed
Details
Mixed content No mixed content detected
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
@maggyero maggyero deleted the maggyero:patch-1 branch Sep 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.