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

chore: update js-doc throughout the app, batch 1 #5309

Merged
merged 5 commits into from
Jul 16, 2019

Conversation

kieckhafer
Copy link
Member

@kieckhafer kieckhafer commented Jul 11, 2019

Resolves parts of #5269 & #5267
Impact: major
Type: docs|chore

Issue

We are missing entire jsdoc blocks in some instances, and param / return / description lines of jsdoc in other instances, along with other jsdoc errors, all throughout Reaction. This is the biggest contributor to our eslint warnings.

Solution

Add / fix jsdoc where necessary.
Since there are so many of these warnings, I'm going to split it up into multiple PR's to keep the file count low for easier viewing / less conflicts with open branches.

Breaking changes

None

Testing

  1. Start the app
  2. Make sure it runs
  3. All changes are documentation changes, there should be no effect on the app in general

More detail for what each of these sections should include are available in our Contributing Docs

@kieckhafer kieckhafer changed the title [WIP] chore: update js-doc throughout the app [WIP] chore: update js-doc throughout the app, batch 1 Jul 12, 2019
@kieckhafer kieckhafer marked this pull request as ready for review July 12, 2019 05:32
@kieckhafer kieckhafer changed the title [WIP] chore: update js-doc throughout the app, batch 1 chore: update js-doc throughout the app, batch 1 Jul 12, 2019
Copy link
Contributor

@von-steinkirch von-steinkirch left a comment

Choose a reason for hiding this comment

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

Erik, I run your branch and the warnings are down to 417! Almost half, very cool.

* @method
* @memberof Shop/GraphQL
* @summary Gets the primary shop
* @param {Object} parentObject - unused
* @param {Object} args - unused
* @param {Object} _ - unused
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, curious what's the meaning of this change (and what does having "_" mean).

Copy link
Member Author

Choose a reason for hiding this comment

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

@bt3gl we have all our queries / mutations set up to accept these three params: parentResult, inputArgs, and context. In a lot of cases, we don't need to pass in parentResult or any args, so we use _ and __ as placeholders for these fields.

@kieckhafer kieckhafer merged commit 9d1af7e into develop Jul 16, 2019
@kieckhafer kieckhafer deleted the chore-kieckhafer-eslint-jsdoc branch July 16, 2019 15:10
@kieckhafer kieckhafer mentioned this pull request Aug 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants