Move period mark after parentheses by etanot · Pull Request #1445 · progit/progit2

etanot · 2020-06-26T21:58:41Z

@ben can you review this pull request, also?

Closes: #1443

Morganov · 2020-06-27T07:27:45Z

book/09-git-and-other-scms/sections/client-bzr.asc


-* git push origin :branch-to-delete (Bazaar can't accept ref deletions in this way.)
+* git push origin :branch-to-delete (Bazaar can't accept ref deletions in this way)
 * git push origin old:new (it will push 'old')


Suggested change

* git push origin old:new (it will push 'old')

* git push origin old:new (it will push `old`)

@Morganov,
I've covered this same snippet of code in different two PR, sorry about that (I didn't notice until now). Here I'm agree with your suggestion, but can you take a look into this suggestion or wait for @ben's reply? So that I've to do changes at only one place.

Let's wait for @ben's reply.

Morganov · 2020-06-27T07:28:34Z

book/09-git-and-other-scms/sections/client-p4.asc

 #  Description: Comments about the changelist.  Required.
 #  Jobs:        What opened jobs are to be closed by this changelist.
-#               You may delete jobs from this list.  (New changelists only.)
+#               You may delete jobs from this list  (new changelists only).


Suggested change

# You may delete jobs from this list (new changelists only).

# You may delete jobs from this list (new changelists only).

Morganov · 2020-06-27T07:28:46Z

book/09-git-and-other-scms/sections/client-p4.asc

 #  Files:       What opened files from the default changelist are to be added
-#               to this changelist.  You may delete files from this list.
-#               (New changelists only.)
+#               to this changelist.  You may delete files from this list


Suggested change

# to this changelist. You may delete files from this list

# to this changelist. You may delete files from this list

Morganov · 2020-06-27T07:34:39Z

book/10-git-internals/sections/environment.asc

-This is the program invoked whenever Git needs to ask the user for credentials, which can expect a text prompt as a command-line argument, and should return the answer on `stdout`.
-(See <<ch07-git-tools#_credential_caching>> for more on this subsystem.)
+This is the program invoked whenever Git needs to ask the user for credentials, which can expect a text prompt as a command-line argument, and should return the answer on `stdout`
+(see <<ch07-git-tools#_credential_caching>> for more on this subsystem).


Please, keep the following convention: "one sentence - one line". Therefore, either concatenate both lines to a single one or remove parentheses and leave the second line as an individual sentence.

Okay, I'll merge it in a single line. @Morganov I've a question, I'm agree with all of your suggestions. But how do I incorporate these suggestions into PR? Means should I use Github Commit suggestions feature? or I've to make these changes locally, then force push it; so you don't have to rebase the PR while merging.

In this particular case, I recommend you do the changes locally, amend the commit, and do the force push.
Here, I've used commit suggestions to show what exactly should be changed only.
IMHO, nobody wants to see the changes, that have been reverted in the same PR (unless it's necessary 😄).

So again, in the matter to keep git history in a cleaner state, I recommend you update the current commit.

If you don't know how to do this, it's OK, go ahead to apply suggestions - all commits in your PR could be squashed automatically on merge.

@dexter has suggested to put branch name inside backtick[1]. [1]: progit#1445 (comment)

@Morganov

@Morganov has suggested to put branch name inside backtick[1]. [1]: progit#1445 (comment)

etanot · 2020-06-27T15:20:13Z

@Morganov I've completed all suggested changes. Can you review it once, again?

ben

This is a great scrub of how we're using parentheses, thanks a ton! I just had one niggle and one small tear to shed, but otherwise it looks great.

ben · 2020-06-29T21:37:08Z

book/09-git-and-other-scms/sections/client-p4.asc

-#               You may delete jobs from this list.  (New changelists only.)
+#               You may delete jobs from this list (new changelists only).
 #  Files:       What opened files from the default changelist are to be added
-#               to this changelist.  You may delete files from this list.
-#               (New changelists only.)
+#               to this changelist. You may delete files from this list
+#               (new changelists only).


It doesn't makes sense to "fix" this, since it's verbatim output from a program we didn't write.

it's verbatim output from a program

Sorry, I missed that. It seems like using period before parentheses is very common, I've seen this in git(1) man page and other places too. Is it a right thing to do (move period after parentheses)?

ben · 2020-06-29T21:45:06Z

book/B-embedding-git/sections/libgit2.asc

 ----

-_(Note that errors are captured, but not handled. We hope your code is better than ours.)_
+_Note that errors are captured, but not handled. We hope your code is better than ours._


I'm a tiny bit sad to see this go. The parens add something. sigh I guess it's the right thing though.

Usually period mark is present after parentheses, see[1] for more information. [1]: https://brians.wsu.edu/2016/05/30/parentheses/ Resolves: progit#1443

@Morganov

@Morganov has suggested to put branch name inside backtick[1]. [1]: progit#1445 (comment)

etanot · 2020-06-30T00:55:14Z

@ben can you review it, once again? I've made suggested change.

ben

Looks great! Thanks!

Morganov suggested changes Jun 27, 2020

View reviewed changes

etanot pushed a commit to etanot/progit2 that referenced this pull request Jun 27, 2020

[enh] put branch name inside backtick

572a1e0

@dexter has suggested to put branch name inside backtick[1]. [1]: progit#1445 (comment)

etanot force-pushed the fix/period-before-parentheses branch from ef5784d to 572a1e0 Compare June 27, 2020 15:14

etanot pushed a commit to etanot/progit2 that referenced this pull request Jun 27, 2020

[enh] put branch name inside backtick

ab00cbe

@Morganov has suggested to put branch name inside backtick[1]. [1]: progit#1445 (comment)

etanot force-pushed the fix/period-before-parentheses branch from 572a1e0 to ab00cbe Compare June 27, 2020 15:17

ben reviewed Jun 29, 2020

View reviewed changes

Vipul Kumar added 2 commits June 30, 2020 00:48

[Fix] move period mark after parentheses

5afed08

Usually period mark is present after parentheses, see[1] for more information. [1]: https://brians.wsu.edu/2016/05/30/parentheses/ Resolves: progit#1443

[enh] put branch name inside backtick

17a5da7

@Morganov has suggested to put branch name inside backtick[1]. [1]: progit#1445 (comment)

etanot force-pushed the fix/period-before-parentheses branch from ab00cbe to 17a5da7 Compare June 30, 2020 00:52

ben approved these changes Jun 30, 2020

View reviewed changes

ben merged commit 2fdc5dc into progit:master Jun 30, 2020

	* git push origin old:new (it will push 'old')
	* git push origin old:new (it will push `old`)

	# You may delete jobs from this list (new changelists only).
	# You may delete jobs from this list (new changelists only).

	# to this changelist. You may delete files from this list
	# to this changelist. You may delete files from this list

Conversation

etanot commented Jun 26, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

etanot commented Jun 27, 2020

Uh oh!

ben left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

etanot commented Jun 30, 2020

Uh oh!

ben left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants