Skip to content

Conversation

@lumirlumir
Copy link
Member

@lumirlumir lumirlumir commented Nov 24, 2024

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[X] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

Hello, I've added missing punctuation in governance.md.

There is always punctuation at the end of the lists (represented with * in Markdown), as shown in the screenshot below. However, the items I added were missing it.

image

Is there anything you'd like reviewers to focus on?

* Technical direction
* Project governance and process (including this policy)
* Contribution policy
* GitHub repository hosting

The sentences in the list above are incomplete, so it’s possible to omit the periods. However, since periods are used consistently throughout the Markdown page, I’m debating whether to include them here as well.

@lumirlumir lumirlumir requested a review from a team as a code owner November 24, 2024 15:38
@eslint-github-bot eslint-github-bot bot added the documentation Relates to ESLint's documentation label Nov 24, 2024
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 24, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@netlify
Copy link

netlify bot commented Nov 24, 2024

Deploy Preview for docs-eslint ready!

Name Link
🔨 Latest commit a0cebcc
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/674d533691f0e500081f73c2
😎 Deploy Preview https://deploy-preview-19161--docs-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Tanujkanti4441 Tanujkanti4441 added the accepted There is consensus among the team that this change meets the criteria for inclusion label Nov 25, 2024
Copy link
Contributor

@Tanujkanti4441 Tanujkanti4441 left a comment

Choose a reason for hiding this comment

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

Hi @lumirlumir, thanks for the PR!

There is inconsistency regarding full stop punctuation in docs pages like:

We have full stop here in list
Screenshot 2024-11-25 190356

but not here
Screenshot 2024-11-25 190333

And there are few more place where punctuation is not consistent in the same list such as:

in manage-issues page.
Screenshot 2024-11-25 190206

or in work-on-issue page
Screenshot 2024-11-25 190139

i think we should also make it consistent on other pages.

@lumirlumir
Copy link
Member Author

Hello, @Tanujkanti4441

Thank you for the detailed review!

I've actually been checking for other missing punctuation in the documentation. If that's okay, would it be alright to add the missing punctuation to all the other document pages as well? I've noticed that many sentences in the lists are missing punctuation.

So far, I've reviewed about a third of the documents and found missing punctuation in more than 10 pages. By the time I'm done, I estimate that there could be issues in over 20 pages.

Now, I'm wondering how best to proceed with this task:

  1. Find all the missing punctuation, integrate them, and update this PR. However, this might make the PR too large to review.
  2. Split the process by category directories (e.g., "INTEGRATE ESLINT," "CONTRIBUTE TO ESLINT," etc.).
  3. ...Other suggestions...

It would be helpful to get your advice on this. I'll follow your recommendation!

Once the direction is decided, I'll let you know when I expect to finish this work.

@Tanujkanti4441
Copy link
Contributor

Now, I'm wondering how best to proceed with this task:

1. Find all the missing punctuation, integrate them, and update this PR. However, this might make the PR too large to review.

2. Split the process by category directories (e.g., "INTEGRATE ESLINT," "CONTRIBUTE TO ESLINT," etc.).

3. ...Other suggestions...

IMHO, it's better to update this PR because on the basis of what i have noticed in docs i am assuming even the number of pages would be large but the content to update (adding punctuation) won't be that much in per page.

But still let's ask other @eslint/eslint-team members that how they would like the PR to be made.

@nzakas
Copy link
Member

nzakas commented Nov 25, 2024

I think it's fine to update this PR will all of the changes.

Just note that not all bullets required periods, only those that are complete sentences.

@lumirlumir
Copy link
Member Author

Thanks, @Tanujkanti4441 and @nzakas.

I think I can finish this by the end of the week. Until then, I'll keep it as a draft.

Once the process is complete, I'll ping the reviewers!

@lumirlumir lumirlumir marked this pull request as draft November 25, 2024 16:31
@lumirlumir lumirlumir changed the title docs: add missing punctuation in governance.md docs: add missing punctuation in document Nov 26, 2024
@lumirlumir lumirlumir marked this pull request as ready for review November 27, 2024 16:26
@lumirlumir
Copy link
Member Author

lumirlumir commented Nov 27, 2024

Hello, @Tanujkanti4441 @nzakas,

I've added as many missing punctuation marks as I could.

I only added them to complete sentences, not to incomplete ones.

Additionally, the Yes. in the screenshot below uses a . while the others don’t. For consistency, I deleted it.

image

@lumirlumir lumirlumir requested a review from nzakas November 27, 2024 16:31
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@nzakas nzakas merged commit e6b84f5 into eslint:main Dec 2, 2024
24 of 25 checks passed
@lumirlumir lumirlumir deleted the patch-1 branch December 2, 2024 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accepted There is consensus among the team that this change meets the criteria for inclusion documentation Relates to ESLint's documentation

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

3 participants