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

Reduce redundancy in story description #1251

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

Conversation

udyzr
Copy link

@udyzr udyzr commented May 10, 2019

Prior to this fix the description insinuates John had made two commits
in this process phase. Please compare with elaborations shortly
before first transcript for John's machine.

Prior to this fix the description insinuates John had made two commits
in this process phase. Please compare with elaborations shortly
before first transcript for John's machine.
@ben
Copy link
Member

ben commented May 11, 2019

The prose had just finished showing how Jessica makes changes, commits them, and pushes them (in painful detail). The line you're trying to change is where we switch back to John. We need to tell his part of the story quickly, and having just gone through all this detail with Jessica, we can shorthand it with John by just saying what he does.

But the change you're suggesting removes the fact that John makes changes at all, and it seems like that's important information. Can you explain a bit more why you think this change is necessary?

@udyzr
Copy link
Author

udyzr commented May 12, 2019

But the change you're suggesting removes the fact that John makes changes at all, and it seems like
that's important information. Can you explain a bit more why you think this change is necessary?

I am not sure this. With this phrase left in text as it is John edits then commits twice - I am not sure that is the intended scenario. Please compare with line 112 as stated in issue #1251. Both line 159 and 112 use present tense, so in my understanding John performs these two operations twice.

Kindly see also #1249 (comment).

@udyzr
Copy link
Author

udyzr commented May 12, 2019

Cit.
The first developer, John, clones the repository, makes a change, and commits locally. (The protocol messages have been replaced with …​ in these examples to shorten them somewhat.)
< here about what Jessica makes >
Continuing with this example, shortly afterwards, John makes some changes, commits them to his local repository, and tries to push them....

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.

Ah I see, I was misreading the content. I thought John pushed after his first set of changes. So the core of your proposal is a good one, but I think we can do better for how it gets clarified.

Let's also change line 128 to say something like this:

While John is busy making his changes, the second developer (Jessica) does the same thing -- clones the repository and commits a change:

@@ -156,7 +156,7 @@ The basic format is `<oldref>..<newref> fromref -> toref`, where `oldref` means
You'll see similar output like this below in the discussions, so having a basic idea of the meaning will help in understanding the various states of the repositories.
More details are available in the documentation for https://git-scm.com/docs/git-push[git-push].

Continuing with this example, shortly afterwards, John makes some changes, commits them to his local repository, and tries to push them to the same server:
Continuing with this example, shortly afterwards, John tries to push them to the same server:
Copy link
Member

Choose a reason for hiding this comment

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

Let's reword this sentence thus:

Continuing with our example, John then tries to push his local changes to the same server:

Copy link
Author

Choose a reason for hiding this comment

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

Well, however myself wonders why phrase ", shortly afterwards, John ... tries" was able to work before change in PR but it isn't with PR.

Copy link
Author

@udyzr udyzr May 14, 2019

Choose a reason for hiding this comment

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

From current point of view I am neither able nor ready to defend this refinement if one day it should need that. My lead rule is to cut as much as necessary as well as as little as possible. Minimal possible increments.

Copy link
Member

Choose a reason for hiding this comment

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

OK, let's stick to just your suggestions then. This isn't quite grammatically correct, let's change it to this:

Continuing with this example, shortly afterwards, John tries to push his changes to the same server:

@udyzr
Copy link
Author

udyzr commented May 13, 2019

Let's also change line 128 to say something like this:

While John is busy making his changes, the second developer (Jessica) does the same thing -- clones the repository and commits a change:

Actually I am happy with book's every piece of text else I do not complain about. Change this type would rather form a side-effect for this issue "Reduce redundancy in story description". Kindly consider an issue for this change - shouldn't it need a good argumentation as every issue else - as well myself doen't share the need of change this kind?
Furthermore I believe also in the end of the day it does not matter if Jessica and John are doing their changes parallel or subsequently in time.

* Fix: Inspecting commits made on topic branch since...

common ancestor with main branch.

Issue detected and fixed by consulting git-revisions manual page
(used git version is 2.21.0):
 <rev1>...<rev2> - Include commits that are reachable from either <rev1> or <rev2>
 but exclude those that are reachable from both...
 <rev1>..<rev2> - Include commits that are reachable from <rev2>
 but exclude those that are reachable from <rev1>...

* Fix referring to

* Fix reference to 'double-dot'
@ben
Copy link
Member

ben commented May 14, 2019

Change this type would rather form a side-effect for this issue "Reduce redundancy in story description".

I appreciate that you're trying to keep this PR focused, but I'm looking at the broader context of how we make this section more readable and useful. Your original suggestion is definitely a part of that, but I think we can do more, and this is the most on-topic place to do it.

Kindly consider an issue for this change - shouldn't it need a good argumentation as every issue else - as well myself doen't share the need of change this kind?

I don't think that every sentence needs a lengthy discussion. That's actually my role here, to make these quick decisions that probably no one has a strong opinion about. If you don't agree with this suggestion that's completely okay, I'm happy to accept your contribution the way you'd like it to look.

Furthermore I believe also in the end of the day it does not matter if Jessica and John are doing their changes parallel or subsequently in time.

For me it helps to visualize two people working on the code at the same time, and one of them pushes first. This doesn't matter on a technical level, but since this is in the part of the book where we're just introducing multi-person workflows, it helps to be as friendly as possible.

Well, however myself wonders why phrase ", shortly afterwards, John ... tries" was able to work before change in PR but it isn't with PR.

I actually think it doesn't work very well.

@udyzr
Copy link
Author

udyzr commented May 14, 2019

Kindly ask person who sees the need for additional change as pointed to put it into repository. Myself either still didn't review text from this perspective or does not see the need for this adaption. I can raise PR for changes myself is convinced to be needed. Also if proposed additional change should receive some critics in future I am not ready to defend it cause I don't understand its need. I appreciate your understanding.

For me it helps to visualize two people working on the code at the same time

Even though. They two might have it (cloning, editing then committing) done literally at the same time or each after another one in very short time distance. This however does not matter. It is the order in which their's two pushes are made which matters imho.
Should "While John is busy making his changes, the second developer" get some critics in future I am neither able nor ready to defend it. Please understand.

I actually think it doesn't work very well.

OK. To understand, to be convinced from as well as to follow I need background.

Ud Yzr added 2 commits May 14, 2019 21:30
referring to a branch in same repository.
It is risky to use term `upstream` when referring
to same repository's branch as this book seems
to not define `upstream` in such coverage.

Let's the elaboration to lack argumentation until
a firm argumentation is found.
@@ -156,7 +156,7 @@ The basic format is `<oldref>..<newref> fromref -> toref`, where `oldref` means
You'll see similar output like this below in the discussions, so having a basic idea of the meaning will help in understanding the various states of the repositories.
More details are available in the documentation for https://git-scm.com/docs/git-push[git-push].

Continuing with this example, shortly afterwards, John makes some changes, commits them to his local repository, and tries to push them to the same server:
Continuing with this example, shortly afterwards, John tries to push them to the same server:
Copy link
Member

Choose a reason for hiding this comment

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

OK, let's stick to just your suggestions then. This isn't quite grammatically correct, let's change it to this:

Continuing with this example, shortly afterwards, John tries to push his changes to the same server:

Jessica can merge either `origin/master` or `issue54` first -- the order doesn't matter.
The end snapshot will be identical no matter which order she chooses; only the history will be different.
However let's mention the fact in this case to be worth to merge `issue54` first as it will be just fast-forward -
no new commit is created - resulting in version tree bit more clearer.
Copy link
Member

Choose a reason for hiding this comment

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

More grammatical updates:

In this case, she chooses to merge issue54 first, since it will be a fast-forward merge, which will result in a simpler history.

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

2 participants