A few cleanups #163

sivaraam · 2017-12-04T13:42:05Z

These commits add a few cleanups that I saw when skimming through the document. A few might be obvious improvements and a few might be needless churns. Let me know about which ones are good/bad so that I could change them accordingly. Hope this helps.

The name of the subsytem is considered to be 'Git' and not 'git'. There was inconsistency in the document by referring to the subsystem using both 'git' and 'Git'. Consolidate the usages to 'Git' which is generally considered to be the name of the subsystem.

They website is generally called 'GitHub' and not 'github'.

This is done for the sake of consistency. Most of the section names don't have a fullstop at the end. So, ...

The Table of Contents seems to have been out of date with the section titles. So, update the ToC with 'doctoc'.

The example for that exhibits the way to 'prune' remote branches that were deleted upstream wasn't flexible as it relied on the command defaulting to the upstream of the current branch. This might lead the reader into overlooking the flexibility of the `git fetch`. Show that the 'upstream' can be mentioned in the command thus show casing the flexibility of `git fetch`.

It's not good for newbies to start using 'force deletion' when they want to delete a branch as it might lead them to them into 'accidentally' deleting their branches often without merging them into other branches or pushing them to an upstream. So, exemplify the safer version of branch deletion (branch -d) and warn them about what `git branch -D` does.

It's not required to use 'git' a lot as this a document about Git, after all.

Rebasing fast-forwards when the tip of the branch is a descendent of the tip of the upstream. In other cases it re-writes the history. This re-write is what actually leads the user to 'force' update the remote. So, clarify that a user has to force update only when the history is re-written regardless of whether the branch was fast-forwarded.

RichardLitt

Pretty good. Did you manually change all of the TOC links? They're supposed to refer to the link anchor tags, although I can't recall exactly why we went for anchor tags instead of just using Markdown.

RichardLitt · 2017-12-04T14:28:38Z

README.md


 ```sh
-$ git fetch -p
+$ git fetch -p upstream


We should add a note saying to check that upstream is the remote you want to fetch from.

RichardLitt · 2017-12-04T14:29:09Z

README.md

+(master)$ git branch -d my-branch
+```
+
+To delete a local branch that HAS NOT been merged to the current branch or an upstream:


No need for all caps for HAS NOT

RichardLitt · 2017-12-04T14:29:53Z

README.md

@@ -729,7 +735,7 @@ $ git push -u origin HEAD

 If you want to check out the other default configs which ```git push``` can take, visit the documentation for Git at https://git-scm.com/docs/git-config#git-config-pushdefault

-If you do not want to change the git configuration, you can also use:
+If you do not want to change the configuration, you can also use:


perhaps "the global configuration for Git" would be more clear?

I think it gives the wrong view. We didn't use the --global option in the previous command. So, we can't say "global configuration". Why do you think "... to change the configuration... " isn;t clear enough?

Regardless, reading the context a little I guess it's confusing readers a lot by using inconsistent terms. I'll try to fix the confusion in a commit. Let's see how it goes.

Capitalizing words seems to be over emphasizing words. Italicize the words, instead to see if works.

sivaraam · 2017-12-06T17:04:02Z

@RichardLitt Thanks for the detailed review.

Did you manually change all of the TOC links?

Nope. I used doctoc (version 1.3.0) as mentioned in my commit message. Any issues?

The commands were needlessly complex by not considering the fact that the command defaults to HEAD when no branch is specified and changing configuration when it wasn't required. Simplify the commands to make readers more happy!

RichardLitt

Close! Getting there!

RichardLitt · 2017-12-06T17:56:50Z

README.md

 ```

+where, `upstream` is the e remote you want to fetch from.


Spelling error?

@sivaraam No need to comment - just amend the commit or add a new one and my comment will go away. :)

RichardLitt · 2017-12-06T17:59:24Z

README.md

 ```

-With the ```upstream``` mode and the ```simple``` mode (default in Git 2.0), the following command will push the current branch w.r.t. the remote branch that has been registered previously with -u :
+With the `upstream` mode and the `simple` (default in Git 2.0) mode of the `push.default` config, the following command will push the current branch w.r.t. the remote branch that has been registered previously with -u :


While we're here, change w.r.t. to 'with regards to', and put ticks around -u, and move the colon next to it?

eamanu · 2017-12-06T18:37:21Z

I like the changes. Please make the requested changed by @RichardLitt

This is an instance of a carelessly edited document getting into version control. ;)

... by, - expanding acronyms - quoting a command line parameter

RichardLitt · 2017-12-08T16:47:44Z

Thank you!

These commands had been inserted in k88hudson#148 and (over)simplified in k88hudson#163. The issue is that `git push <remote>` & `git push -u <remote` will typically raise the error below (unlike `git push <remote> HEAD`): --8<---------------cut here---------------start------------->8--- fatal: The current branch new-branch has no upstream branch. To push the current branch and set the remote as upstream, use git push --set-upstream origin new-branch --8<---------------cut here---------------end--------------->8---

These commands had been inserted in k88hudson#148 and (over)simplified in k88hudson#163. The issue is that `git push <remote>` and `git push -u <remote>` will typically raise the error below (unlike `git push <remote> HEAD`): --8<---------------cut here---------------start------------->8--- fatal: The current branch new-branch has no upstream branch. To push the current branch and set the remote as upstream, use git push --set-upstream origin new-branch --8<---------------cut here---------------end--------------->8---

#208) * Add missing HEAD argument in 2 Git commands These commands had been inserted in #148 and (over)simplified in #163. The issue is that `git push <remote>` and `git push -u <remote>` will typically raise the error below (unlike `git push <remote> HEAD`): --8<---------------cut here---------------start------------->8--- fatal: The current branch new-branch has no upstream branch. To push the current branch and set the remote as upstream, use git push --set-upstream origin new-branch --8<---------------cut here---------------end--------------->8--- * Add doc URL & Avoid triple backquotes

* Consolidate the name of the subsystem The name of the subsytem is considered to be 'Git' and not 'git'. There was inconsistency in the document by referring to the subsystem using both 'git' and 'Git'. Consolidate the usages to 'Git' which is generally considered to be the name of the subsystem. * Consolidate the name of GitHub They website is generally called 'GitHub' and not 'github'. * Quote the git commands correctly * Avoid fullstops in section names This is done for the sake of consistency. Most of the section names don't have a fullstop at the end. So, ... * Update the ToC The Table of Contents seems to have been out of date with the section titles. So, update the ToC with 'doctoc'. * Clarify that the changes are removed only for the previous commit * Showcase the flexibility of `git fetch -p` The example for that exhibits the way to 'prune' remote branches that were deleted upstream wasn't flexible as it relied on the command defaulting to the upstream of the current branch. This might lead the reader into overlooking the flexibility of the `git fetch`. Show that the 'upstream' can be mentioned in the command thus show casing the flexibility of `git fetch`. * Exemplify the safer version of branch deletion It's not good for newbies to start using 'force deletion' when they want to delete a branch as it might lead them to them into 'accidentally' deleting their branches often without merging them into other branches or pushing them to an upstream. So, exemplify the safer version of branch deletion (branch -d) and warn them about what `git branch -D` does. * Improve readability of a few phrases It's not required to use 'git' a lot as this a document about Git, after all. * Use HTTPS links for sites that serve using HTTPS * Clarify that rebasing just re-writes history Rebasing fast-forwards when the tip of the branch is a descendent of the tip of the upstream. In other cases it re-writes the history. This re-write is what actually leads the user to 'force' update the remote. So, clarify that a user has to force update only when the history is re-written regardless of whether the branch was fast-forwarded. * Attribute both the authors of the second edition of Pro Git * Try a different form of emphasizing text Capitalizing words seems to be over emphasizing words. Italicize the words, instead to see if works. * Mention what 'upstream' means for the sake of clarity * Simplify the way to create a remote branch from the local one The commands were needlessly complex by not considering the fact that the command defaults to HEAD when no branch is specified and changing configuration when it wasn't required. Simplify the commands to make readers more happy! * Remove a character cruft left over while editing This is an instance of a carelessly edited document getting into version control. ;) * Improve a sentence ... by, - expanding acronyms - quoting a command line parameter

k88hudson#208) * Add missing HEAD argument in 2 Git commands These commands had been inserted in k88hudson#148 and (over)simplified in k88hudson#163. The issue is that `git push <remote>` and `git push -u <remote>` will typically raise the error below (unlike `git push <remote> HEAD`): --8<---------------cut here---------------start------------->8--- fatal: The current branch new-branch has no upstream branch. To push the current branch and set the remote as upstream, use git push --set-upstream origin new-branch --8<---------------cut here---------------end--------------->8--- * Add doc URL & Avoid triple backquotes

sivaraam added 12 commits December 4, 2017 18:16

Consolidate the name of GitHub

43ae80e

They website is generally called 'GitHub' and not 'github'.

Avoid fullstops in section names

be8bb4b

This is done for the sake of consistency. Most of the section names don't have a fullstop at the end. So, ...

Update the ToC

6c64e0f

The Table of Contents seems to have been out of date with the section titles. So, update the ToC with 'doctoc'.

Quote the git commands correctly

f19a94c

Clarify that the changes are removed only for the previous commit

d1fa7e3

Improve readability of a few phrases

ed04605

It's not required to use 'git' a lot as this a document about Git, after all.

Use HTTPS links for sites that serve using HTTPS

b824580

Attribute both the authors of the second edition of Pro Git

ce61b09

RichardLitt reviewed Dec 4, 2017

View reviewed changes

sivaraam added 2 commits December 6, 2017 22:14

Try a different form of emphasizing text

8fb8200

Capitalizing words seems to be over emphasizing words. Italicize the words, instead to see if works.

Mention what 'upstream' means for the sake of clarity

50a5eea

RichardLitt requested changes Dec 6, 2017

View reviewed changes

sivaraam added 2 commits December 8, 2017 22:12

Remove a character cruft left over while editing

3765a67

This is an instance of a carelessly edited document getting into version control. ;)

Improve a sentence

e2f230a

... by, - expanding acronyms - quoting a command line parameter

RichardLitt merged commit 12bc633 into k88hudson:master Dec 8, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

A few cleanups #163

A few cleanups #163

sivaraam commented Dec 4, 2017

RichardLitt left a comment

RichardLitt Dec 4, 2017

RichardLitt Dec 4, 2017

RichardLitt Dec 4, 2017

sivaraam Dec 6, 2017

sivaraam commented Dec 6, 2017

RichardLitt left a comment

RichardLitt Dec 6, 2017

sivaraam Dec 8, 2017

RichardLitt Dec 8, 2017

RichardLitt Dec 6, 2017

eamanu commented Dec 6, 2017

RichardLitt commented Dec 8, 2017

		```

		where, `upstream` is the e remote you want to fetch from.

A few cleanups #163

A few cleanups #163

Conversation

sivaraam commented Dec 4, 2017

RichardLitt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sivaraam commented Dec 6, 2017

RichardLitt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eamanu commented Dec 6, 2017

RichardLitt commented Dec 8, 2017