Skip to content
This repository has been archived by the owner on Sep 24, 2022. It is now read-only.

Added hyphens to make it in sync with the current writing style #139

Merged
merged 3 commits into from
Sep 30, 2020
Merged

Added hyphens to make it in sync with the current writing style #139

merged 3 commits into from
Sep 30, 2020

Conversation

Dishebh
Copy link
Contributor

@Dishebh Dishebh commented May 24, 2020 •
edited
Loading

Changes made:

  • Added hyphens in phrases / words Step by step and Lead in.

Resolves #141

@Dishebh
Copy link
Contributor Author

Dishebh commented May 24, 2020

@jaredmorgs @camerons Please review :)

@camerons
Copy link
Member

@Dishebh, thanks for this pull request. It looks likely that this is a valid improvement.
But, I can't see a reference to an issue that you are fixing. (I might be missing something, I'm not an expert on github).
It is good practice with all fixes to:

  1. Create an issue.
  2. Explain why you are making the fix, ideally referencing a document which backing up your reasoning. In this case, you'd be referencing a section in the Google Developers Style Guide.
  3. Noting whether you have fixed it just in this document, or whether you have reviewed all templates for this rule you are fixing.
    By doing this, you help the reviewers, (and lack of reviewers is often a bottleneck in getting pull requests completed.)
    Cheers, Cameron

@Dishebh
Copy link
Contributor Author

Dishebh commented May 25, 2020

@camerons since it was a small change, I did not think of opening an issue for it. But your reasoning makes perfect sense, and how the whole process of issue -> discuss -> PR -> review -> merge makes it easier to track the changes in repo. I'll open an issue for it and refer it in the PR description.

jaredmorgs
jaredmorgs approved these changes May 25, 2020
View reviewed changes
Copy link
Contributor

@jaredmorgs jaredmorgs left a comment

Choose a reason for hiding this comment

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

In the spirit of Iterate, don't gold plate I'm happy for this to be merged providing you fix up that instance of "E.g:" that crept into this file somehow (not introduced by you @Dishebh , but would be good to fix while we're in this file).

Thanks again for following the due process with this issue @Dishebh and creating that issue.

how-to/template-how-to.md Outdated
@@ -36,7 +36,7 @@ Lead-in sentence for an ordered list:

### Step 3: Optional: title for step - code snippet

Lead in sentence explaining the code snippet. E.g.:
Lead-in sentence explaining the code snippet. E.g.:
Copy link
Contributor

Choose a reason for hiding this comment

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

I know it is not part of the core issue here, but E.g.: is not recommended according to https://developers.google.com/style/abbreviations#dont-use

Could you please change this to:

"For example: "

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replaced abbreviation by its full-form. Glad that I could help @jaredmorgs :)

@shreyas1599
Copy link
Contributor

@Dishebh perhaps you could edit this PR and others that you've opened and link the relevant issues so that they get closed automatically after merging.

@Dishebh
Copy link
Contributor Author

Dishebh commented May 25, 2020

@shreyas1599, sure. I edited this PR description and will make similar changes in my other PRs as well.

@Dishebh Dishebh requested a review from jaredmorgs June 26, 2020 05:51
@jaredmorgs jaredmorgs merged commit 7e72d0d into thegooddocsproject:master Sep 30, 2020
@Dishebh Dishebh deleted the style branch September 30, 2020 11:30
camerons pushed a commit to camerons/templates that referenced this pull request Feb 27, 2021
@Dishebh
Added hyphens to make it in sync with the current writing style (theg…
83b60e3 
…ooddocsproject#139)

* added hyphens to make it in sync with the current writing style

* replaced abbreviation by its full-form
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Loquacity Loquacity Loquacity approved these changes

@jaredmorgs jaredmorgs Awaiting requested review from jaredmorgs

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Replace some terms with their hyphenated version to make it in sync with the current style guide
5 participants
@Dishebh @camerons @shreyas1599 @jaredmorgs @Loquacity