Skip to content

Conversation

@noahtallen
Copy link
Contributor

@noahtallen noahtallen commented Jan 19, 2023

Proposed Changes

Split from #72308, this PR would enforce capitalized types for Object in jsdoc. E.g.:

/*
 * @param {Array} foo Some list
 * @returns {Object} Some object
/*
function arrToObj( foo ) {
  return {};
}

Note that Array is capitalized by default.

Why? This is what WordPress coding style uses, because it's what the JSdoc standard uses. Thankfully, with a minimal rule change, this is completely autofixable. And if you accidentally write a lower-case variant, it should be auto-fixed in the editor if you lint on save :)

The counterpoint to this is the documentation here: https://github.com/gajus/eslint-plugin-jsdoc/tree/v39.0.0#why-not-capital-case-everything.

Testing Instructions

Verify code and rule changes make sense.

@noahtallen noahtallen added the [Type] Tooling Related to tools, scripts, automation, DevX, etc. label Jan 19, 2023
@noahtallen noahtallen self-assigned this Jan 19, 2023
@noahtallen noahtallen requested review from a team and worldomonation as code owners January 19, 2023 20:20
@noahtallen noahtallen requested a review from a team January 19, 2023 20:20
@noahtallen noahtallen requested review from a team as code owners January 19, 2023 20:20
@noahtallen noahtallen requested a review from a team January 19, 2023 20:20
@noahtallen noahtallen requested a review from a team as a code owner January 19, 2023 20:20
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jan 19, 2023
@github-actions
Copy link

github-actions bot commented Jan 19, 2023

@matticbot
Copy link
Contributor

This PR modifies the release build for editing-toolkit

To test your changes on WordPress.com, run install-plugin.sh editing-toolkit capitalize-jsdoc-arrays-objects on your sandbox.

To deploy your changes after merging, see the documentation: PCYsg-mMA-p2

Copy link
Contributor

@WunderBart WunderBart left a comment

Choose a reason for hiding this comment

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

Fastest ~3k lines review ever 🚢 😎

Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

🚢 Thank you!

I almost missed this as I was reviewing #72308 first.

I appreciate it that we maintain consistency with Gutenberg's codebase.

🚀

@noahtallen noahtallen force-pushed the capitalize-jsdoc-arrays-objects branch from 8b21ce0 to 8af0c6e Compare January 20, 2023 23:09
@matticbot
Copy link
Contributor

This PR modifies the release build for happy-blocks

To test your changes on WordPress.com, run install-plugin.sh happy-blocks capitalize-jsdoc-arrays-objects on your sandbox.

To deploy your changes after merging, see the documentation: PCYsg-r7r-p2

@matticbot
Copy link
Contributor

This PR modifies the release build for wpcom-block-editor

To test your changes on WordPress.com, run install-plugin.sh wpcom-block-editor capitalize-jsdoc-arrays-objects on your sandbox.

To deploy your changes after merging, see the documentation: PCYsg-l4k-p2

@noahtallen noahtallen force-pushed the capitalize-jsdoc-arrays-objects branch from 8af0c6e to d6c8a5a Compare January 20, 2023 23:19
@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@noahtallen noahtallen force-pushed the capitalize-jsdoc-arrays-objects branch 2 times, most recently from 0fd1feb to 7b48568 Compare January 21, 2023 01:10
@noahtallen noahtallen force-pushed the capitalize-jsdoc-arrays-objects branch from 7b48568 to 60f70ce Compare January 21, 2023 01:31
@noahtallen
Copy link
Contributor Author

noahtallen commented Jan 21, 2023

I had to ignore a couple of existing problems in order to get everything passing. But I added @todo comments and links to relevant info in those cases. (There were only two or three of these.) I added all that to 05403f1, and kept the auto fix separate in 60f70ce

@noahtallen
Copy link
Contributor Author

I'll look into why translate is timing out next :)

@noahtallen noahtallen force-pushed the capitalize-jsdoc-arrays-objects branch from 60f70ce to c4e9452 Compare January 23, 2023 22:11
@noahtallen
Copy link
Contributor Author

I'll look into why translate is timing out next :)

I think it's just because the number of files changing is very large. The script loops through every changed file, and then every changed line. But I'll keep an eye on it after merging

Fix other eslint issues in impacted files

Update eslint-plugin-jsdoc again

Set unify paraent child type checks to true
@noahtallen noahtallen force-pushed the capitalize-jsdoc-arrays-objects branch from c4e9452 to 84a1da4 Compare January 24, 2023 00:44
@noahtallen noahtallen force-pushed the capitalize-jsdoc-arrays-objects branch from 84a1da4 to 6ec03cf Compare January 24, 2023 00:51
@noahtallen noahtallen merged commit a14b672 into trunk Jan 24, 2023
@noahtallen noahtallen deleted the capitalize-jsdoc-arrays-objects branch January 24, 2023 01:10
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Tooling Related to tools, scripts, automation, DevX, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants