Skip to content

Conversation

sivaraam
Copy link
Contributor

A few suggestions are required for the following change,

  • In the last code sample found in the Commenting on an Issue subsection of the Scripting GitHub section (just above figure 6-54) has a line like this,
    $ curl -H "Content-Type: application/json" \
     -H "Authorization: token TOKEN" \
      ....

The second line seems to need a token generated using GItHub in place of which the word TOKEN is placed. Wouldn't it be nice to add a note about that in that section?

 * corrected the first line of the chapter to prevent the over stress
   about GitHub
 * changed a sentence to make it read better
 * added "forking" to the GitHub workflow
 * retitled a section from "Markdown" to "GitHub Flavored Markdown" and
   reduced the level of sections under the latter section by 1 as, much
   wasn't told about Markdown in general
 * changed the image referenced to one that's easier to see
 * corrected the sentence about the protocol specified
 * retitled a section from "Hooks" to "Services and Hooks" as it had
   subsections about both
image::images/markdown-08-drag-drop.png[Drag and drop images]

If you look back at <<_pr_references>>, you can see a small ``Parsed as Markdown'' hint above the text area. Clicking on that will give you a full cheat sheet of everything you can do with Markdown on GitHub.
If you at <<_md_drag>>, you can see a small ``Parsed as Markdown'' hint above the text area. Clicking on that will give you a full cheat sheet of everything you can do with Markdown on GitHub.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A verb is missing in the first part of this sentence, the one you removed (look back).

Copy link
Member

@ben ben left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the two things that were called out (thanks, @aollier!). Other than those, this is great, thanks!


(((GitHub)))
GitHub is the single largest host for Git repositories, and is the central point of collaboration for millions of developers and projects.
GitHub is one of the largest host for Git repositories, and is the central point of collaboration for millions of developers and projects.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is incorrect. As of right now, GitHub is still the largest single Git host. We only included a chapter on GitHub because the average reader will most likely be using it.

 * changed back the first sentence to it's original form
 * added back a word that was deleted by mistake
@sivaraam
Copy link
Contributor Author

sivaraam commented Jan 23, 2017

@ben I have made the changes as requested. Any suggestions regarding my initial comment ?

Copy link
Member

@ben ben left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@ben ben merged commit 66d2711 into progit:master Jan 23, 2017
@sivaraam sivaraam deleted the chap-6-changes branch January 24, 2017 13:07
@sivaraam sivaraam restored the chap-6-changes branch January 24, 2017 13:07
@sivaraam
Copy link
Contributor Author

@ben I guess you misunderstood my question. I asked suggestion regarding the following,

In the last code sample found in the Commenting on an Issue subsection of the Scripting GitHub section (just above figure 6-54) has a line like this,

$ curl -H "Content-Type: application/json" \
 -H "Authorization: token TOKEN" \
  ....

The second line seems to need a token generated using GItHub in place of which the word TOKEN is placed. Wouldn't it be nice to add a note about that in that section?

@ben
Copy link
Member

ben commented Jan 24, 2017

Oh yeah, sorry about that. I don't think it's necessary, since the section that line is in spends 4 entire paragraphs walking through generating a token, and says this right before the code snippet:

do an HTTP POST request to repos/<user>/<repo>/issues/<num>/comments with the token we just generated as an Authorization header.

@sivaraam
Copy link
Contributor Author

👍

@sivaraam sivaraam deleted the chap-6-changes branch May 11, 2017 03:37
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.

3 participants