-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Improve JS package descriptions #8121
Conversation
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.
Nice one 💯 Thanks for opening this PR. In general, I like those changes because they bring consistency. I would prefer to have someone else to double check them before we proceed with mthe erge.
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.
I dig it. A few thoughts but this is better than before so 🚢. If you want to make any of the small changes I suggest then feel free to do so before merge 😄
@@ -1,7 +1,7 @@ | |||
{ | |||
"name": "@wordpress/babel-plugin-import-jsx-pragma", | |||
"version": "1.0.1", | |||
"description": "Babel transform plugin for automatically injecting an import to be used as the pragma for the React JSX Transform plugin.", |
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.
Interesting, I'd have thought sentence-case would be better for those. Some descriptions are surely multi-sentence?
It's split on NPM (React seems to be all without periods; lodash all with, etc.)
Given we're in the "comments must have full-stops" camp (I like it), maybe we should be here too? 🤷♂️
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.
Given we're in the "comments must have full-stops" camp (I like it), maybe we should be here too?
I would be also happy to see this change.
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.
Seems like something that would be good to have (though from what I can tell, not yet offered as a feature) from npm-package-json-lint
, which we're already using and have a predefined configuration for.
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.
Hey! I just released v3.3.0 of npm-package-json-lint
with a new description-format
rule. It has two options:
requireCapitalFirstLetter
- Throws an error if the first character in the description isn't capitalized.requireEndingPeriod
- Throws an error if the description doesn't end with a period.
Hopefully this works well for your team!
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.
Thanks, opened PR to start using this new rule: #8596.
@@ -1,7 +1,7 @@ | |||
{ | |||
"name": "@wordpress/components", | |||
"version": "1.0.1", | |||
"description": "UI Compoonents for WordPress", |
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.
😅
packages/compose/package.json
Outdated
@@ -1,7 +1,7 @@ | |||
{ | |||
"name": "@wordpress/compose", | |||
"version": "1.0.1", | |||
"description": "WordPress Higher Order components", | |||
"description": "WordPress higher-order components", |
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.
I'd add "(HOC)" in brackets here; lots of people throw that acronym around and giving newbies context here for free is 👍
packages/keycodes/package.json
Outdated
@@ -1,7 +1,7 @@ | |||
{ | |||
"name": "@wordpress/keycodes", | |||
"version": "1.0.1", | |||
"description": "KeyCodes utilities for WordPress", | |||
"description": "Keycodes utilities for WordPress", |
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.
This one, I think, could be expanded upon further ("Keycodes utilities for WordPress; used to check for keyboard events across browsers/operating systems")
Travis failed after I added fixes suggested by @tofumatt. Those are only text changes so I will merge as is. |
Many of these descriptions are reused as the tagline description in the README.md (which I think is a good thing), and are now not always in sync (e.g. @wordpress/npm-package-json-lint-config). |
Description
I went in to fix a typo in the
@wordpress/components
package description, and ended up giving them all a bit of love.How has this been tested?
It has not been tested — the changes are only to text in the packages'
package.json
files.Types of changes