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
base: main
Are you sure you want to change the base?
Conversation
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.
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? |
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). |
Cit. |
There was a problem hiding this 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: |
There was a problem hiding this comment.
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:
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
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? |
* 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'
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.
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.
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.
I actually think it doesn't work very well. |
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.
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.
OK. To understand, to be convinced from as well as to follow I need background. |
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: |
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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.
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.