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

minor consistency updates to the writing checklist #292

Merged
merged 5 commits into from
Jan 29, 2024

Conversation

nsryan2
Copy link
Member

@nsryan2 nsryan2 commented Dec 11, 2023

Summary of changes

This PR makes the grammar employed in the guide consistent:

  1. Add periods to the end of each check mark.
  2. Make the use of quotes consistent.
  3. Make the voice more consistent.
  4. Capitalize where appropriate.
  5. Fix the spacing in / examples.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Associated Issues and PRs

  • Issue: #

Associated Developers

  • Dev: @

Checklist for Reviewers

Reviewers should use this link to get to the
Review Checklist before they begin their review.

Copy link
Contributor

@smpark7 smpark7 left a comment

Choose a reason for hiding this comment

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

Good job on going through the writing checklist while reviewing the writing checklist. I have some minor comments.

manual/guides/writing/checklist.md Outdated Show resolved Hide resolved
manual/guides/writing/checklist.md Outdated Show resolved Hide resolved
manual/guides/writing/checklist.md Outdated Show resolved Hide resolved
manual/guides/writing/checklist.md Show resolved Hide resolved
manual/guides/writing/checklist.md Outdated Show resolved Hide resolved
manual/guides/writing/checklist.md Outdated Show resolved Hide resolved
```

Here's an example of an equation:
```latex
The line is defined as
The definition of a line is
Copy link
Contributor

Choose a reason for hiding this comment

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

This example is referring to a line, not the general definition of lines. I suggest keeping the original text.

Copy link
Contributor

Choose a reason for hiding this comment

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

On second thought, I'm not so sure.

Copy link
Member Author

@nsryan2 nsryan2 Jan 24, 2024

Choose a reason for hiding this comment

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

The original was passive, but if you're thinking the example is for a specific line we could make it active by saying, "We define this line as"

Copy link
Contributor

Choose a reason for hiding this comment

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

The phrase "... is defined as ..." is so widely used, I wonder if it's exempted from the passive voice rule.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe for a line, because it's a common definition, but I think the context we could use "is defined as"
in our writing has a paper or author we could point to to make it active and not erase that information. All that being said, this example goes against the rule in line 37, so it should be changed from what I put!

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe for a line, because it's a common definition, but I think the context we could use "is defined as"
in our writing has a paper or author we could point to to make it active and not erase that information.

Okay I see your point here.

All that being said, this example goes against the rule in line 37, so it should be changed from what I put!

Applying the rule in line 37 here seems awkward ("definition of a line" -> "line definition"). I'm okay with your original change.

Meta automation moved this from new to in review Jan 23, 2024
@smpark7
Copy link
Contributor

smpark7 commented Jan 23, 2024

If any of the undergrads also want to review this PR, feel free to do so. "The more the merrier" applies for PR reviews.

nsryan2 and others added 2 commits January 23, 2024 22:17
Co-authored-by: Sun Myung Park <park_cg@live.com>
@nsryan2
Copy link
Member Author

nsryan2 commented Jan 24, 2024

Thanks for the feedback! I addressed all your comments except for the one about e.g., I wasn't sure what that one was about.

@nsryan2 nsryan2 requested a review from smpark7 January 24, 2024 04:35
Copy link
Contributor

@smpark7 smpark7 left a comment

Choose a reason for hiding this comment

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

I'll check in again on Friday in case anyone else wants to review this.

@smpark7
Copy link
Contributor

smpark7 commented Jan 29, 2024

Merging this because I see no other review activity.

@smpark7 smpark7 merged commit d3bc02b into arfc:source Jan 29, 2024
Meta automation moved this from in review to completed Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Meta
  
completed
Development

Successfully merging this pull request may close these issues.

None yet

2 participants