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

Gitignore notes and word-based diffing #392

Merged
merged 2 commits into from
Mar 9, 2017

Conversation

junghans
Copy link
Contributor

@junghans junghans commented Mar 7, 2017

  • Added some notes about git:// and ssh proxy
  • Added a note about ** pattern in gitignore section
  • Mention --color-words option in diff section, which is extremely useful for LaTeX files.

@@ -474,6 +474,13 @@ Date: Thu Aug 22 09:51:46 2013 -0400
~~~
{: .output}

> ## Word-based diffing
>
> Sometimes, e.g. in the case of the LaTeX documment a line-wise
Copy link
Contributor

Choose a reason for hiding this comment

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

documment -> document

Copy link
Contributor

@iglpdc iglpdc left a comment

Choose a reason for hiding this comment

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

Thanks @junghans! I made some comments, could you please take a look?

@@ -474,6 +474,13 @@ Date: Thu Aug 22 09:51:46 2013 -0400
~~~
{: .output}

> ## Word-based diffing
>
> Sometimes, e.g. in the case of the LaTeX documment a line-wise
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a missing comma after "document":

"Sometimes, e.g. in the case of a LaTeX document, ..."

but I think it may even better not to mention the LaTeX example: most of our learners do not use it and this may make them think that the color diff is not useful for them.

> ## Word-based diffing
>
> Sometimes, e.g. in the case of the LaTeX documment a line-wise
> diff is too coarse. That is where the `--color-words` option of
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rephrase it to explain a bit what color-words does. From the docs: "Highlights the changed words using colors". What do you think of changing your sentence "That is where ... " to include something similar?

@@ -98,6 +98,8 @@ same commands to choose another editor or update your email address.
> $ git config --global --unset https.proxy
> ~~~
> {: .bash}
> In addition git supports the SSH protocol, which needs no proxy configuration, and
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer not to include this change. I think it's useful information for a more advanced crowd, but it may be confussing for a novice. The only reason the proxy callout is here is because it may be needed in some workshops with a weird network configuration.

@@ -144,6 +144,7 @@ Branch master set up to track remote branch master from origin.
> $ git config --global --unset https.proxy
> ~~~
> {: .bash}
> Please remember that for using SSH as connection, proxy setup is not needed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Something as above. I think mentioning details of SSH won't be useful and may be confussing for most of our learners. We have a few links to set up properly SSH in this episode and I think this is enough.

@junghans junghans changed the title Proxy notes and word-based diffing Gitignore notes and word-based diffing Mar 7, 2017
@junghans
Copy link
Contributor Author

junghans commented Mar 9, 2017

@iglpdc: Done!

@iglpdc
Copy link
Contributor

iglpdc commented Mar 9, 2017

Thanks a lot @junghans !

@iglpdc iglpdc merged commit 95c3c11 into swcarpentry:gh-pages Mar 9, 2017
zkamvar pushed a commit that referenced this pull request May 8, 2023
Add gitignore notes and word-based diffing
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