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

[Docs] - ofBook/chapters/version_control_with_git/chapter.md - fixing document #282

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ghost
Copy link

@ghost ghost commented Jan 13, 2018

[Docs] - ofBook/chapters/version_control_with_git/chapter.md - fixing document

… document

#### [Docs] - ofBook/chapters/version_control_with_git/chapter.md - fixing document
@@ -71,8 +71,8 @@ This means that the contents of that folder are tracked with Git.
Most of the files associated with Git are in the `.git` folder in your project root (the leading dot means this folder is by probably hidden from view in your file browser by default).

The basic element for tracking the history of the repository is the **commit**.
This is basically a snapshot of the repository's state at the time of the commit, including a **commit message** and any parent commit(s).
Think of it as a checkpoint for saving in a videogame.
This is basically a snapshot of the repository's state at the time of the commit, including a **commit message** and any parent commits (s).
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the old version "commit(s)" is correct since the sentence points to a single or maybe multiple commits. A free standing "(s)" has no meaning in this (new) context.


It's important that the Git repository contains all files necessary to successfully compile our program, but no unnecessary stuff.
Generally, this means that files we edit by hand (e.g. source and header files, Readme files, images,...) should be included in the repository, but files which are *generated from* our code (e.g. compiled binaries, pdfs generated from some source file, video files or image sequences created with our program) should stay out.
Generally, this means that files we edit by hand (e.g. source and header files, Readme files, images,...) should be included in the repository, but files which are *generated from* our code (e.g. compiled binaries, pdf generated from some source file, video files or image sequences created with our program) should stay out.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Multiple pdf files may be generated from one source file, so I lean slightly towards the plural form, but only slightly.

@@ -835,7 +834,7 @@ Note that Git tags are only pushed to a remote if you supply the `--tags` flag.

#### Pull requests

A central feature of the Gitub collaboration model are [**pull requests**](https://help.github.com/articles/using-pull-requests).
A central feature of the GitHub collaboration model is [**pull requests**](https://help.github.com/articles/using-pull-requests).
Copy link
Collaborator

Choose a reason for hiding this comment

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

either "are" (as in the old version) or "pull request" for the new version

@tpltnt
Copy link
Collaborator

tpltnt commented Jan 13, 2018

Looks good, only tiny issues to fix.

@ghost
Copy link
Author

ghost commented Jan 13, 2018

@tpltnt i alredy fixed everything you asked for. shall we proceed with merging? cheers

@tpltnt
Copy link
Collaborator

tpltnt commented Jan 14, 2018

They look good to me, but I don't have the permission(s) to merge :(

@@ -160,7 +161,7 @@ Initialised empty Git repository in /home/bilderbuchi/demoProject/.git/

#### `.gitignore`

One thing we should do right at the beginning is add a special Git file called [`.gitignore`](http://git-scm.com/docs/gitignore) to the root of our repository.
One thing we should do right at the beginning adds a special Git file called [`.gitignore`](http://git-scm.com/docs/gitignore) to the root of our repository.
Copy link
Member

Choose a reason for hiding this comment

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

sounds odd. I'd rather say "is to add" or "is adding" instead of "adds"

@@ -170,7 +171,7 @@ If we take care of this right at the beginning, we can easily make sure that onl
Git handles this file exclusion with the aforementioned `.gitignore` files (there can be several), which contains patterns describing which files are ignored by Git.
Those ignored files will still exist in our working directory, that means we can still use them, but Git will not track them.

If, later down the line, we see files appearing in our list of changes which should not be there, or if we can't seem to add a file that belongs in the repository, we don't force Git to do what it doesn't want to, rather fine-tune the `.gitignore` pattern to match our expectations.
If later down the line, we see files appearing in our list of changes which should not be there, or if we can't seem to add a file that belongs in the repository, we don't force Git to do what it doesn't want to, rather fine-tune the `.gitignore` pattern to match our expectations.
Copy link
Member

Choose a reason for hiding this comment

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

I'd say that it was correct with the comma

@@ -347,7 +348,7 @@ We have just made a modification to a file that Git is tracking, so it should pi
```bash
$ git status
# On branch master
# Changes not staged for commit:
# Changes are not staged for commit:
Copy link
Member

Choose a reason for hiding this comment

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

This should be as it was! it is what git outputs.

@@ -369,8 +370,7 @@ It can be used to compare states between all kinds of areas (check out the examp
Let's check it out:

```bash
$ git diff
diff --git a/src/ofApp.cpp b/src/ofApp.cpp
$ git diff --git a/src/ofApp.cpp b/src/ofApp.cpp
Copy link
Member

Choose a reason for hiding this comment

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

same here. It was correct before. it is what thegit diff command will output and not extra arguments for it.

@ghost
Copy link
Author

ghost commented Jan 15, 2018

hey guys. i replaced my previous account by this one, for some issues (too many repositories, et al(. if i can fix push access to this post i will be fixing things as soon as possible. thank you for feedback and for your patience. cheers

@edap
Copy link
Member

edap commented Oct 25, 2018

@roymacdonald @tem44 what's the status of this PR?

@roymacdonald
Copy link
Member

Hi there. I think I missed the notification for @edap 's last comment.
I have merging permissions but I think that, at least the two last comments on code I made ( regarding to the git command) should be fixed before merging. Who was the original author of this PR?

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.

None yet

3 participants