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.

Sign up for GitHub

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

Jump to bottom

Refactor: update all JSDocs #5891

Merged
merged 19 commits into from
May 13, 2022
Merged

Refactor: update all JSDocs #5891

merged 19 commits into from
May 13, 2022

Conversation

queengooborg
Copy link
Contributor

This PR is a subset of #5654 and #5655. This PR extracts all JSDoc-based changes from the original PRs and combines them into this PR. This primarily includes the following changes:

  1. Adding descriptions to the functions
  2. Adding descriptions to function parameters
  3. Adding descriptions to the return values
  4. Probably adding more descriptions somewhere
  5. Updating and fixing JSDoc types, especially ones confused for TypeScript types

@queengooborg queengooborg requested a review from Elchi3 as a code owner March 27, 2020 02:08
@ghost ghost added bulk_update An update to a mass amount of data, or scripts/linters related to such changes linter Issues or pull requests regarding the tests / linter of the JSON files. scripts Issues or pull requests regarding the scripts in scripts/. labels Mar 27, 2020
@queengooborg queengooborg requested a review from ddbeck March 27, 2020 02:09
@queengooborg queengooborg mentioned this pull request Mar 27, 2020
Closed
23 tasks
@ddbeck ddbeck removed their request for review September 8, 2020 14:12
@queengooborg queengooborg requested a review from ddbeck as a code owner October 8, 2020 22:23
@ddbeck ddbeck removed their request for review December 17, 2020 16:23
@Elchi3 Elchi3 removed their request for review February 18, 2021 17:16
Base automatically changed from master to main March 24, 2021 12:54
@queengooborg queengooborg self-assigned this Sep 24, 2021
@queengooborg queengooborg removed their assignment Sep 24, 2021
@github-actions
Copy link

github-actions bot commented May 5, 2022

This pull request has merge conflicts that must be resolved before we can merge this.

@github-actions
Copy link

github-actions bot commented May 7, 2022

This pull request has merge conflicts that must be resolved before we can merge this.

@github-actions
Copy link

github-actions bot commented May 8, 2022

This pull request has merge conflicts that must be resolved before we can merge this.

caugner
caugner requested changes May 12, 2022
View reviewed changes
Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

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

Great initiative, LGTM, just some nits plus some bike-shedding.

PS: This reveals that some functions are really not well-named.

scripts/fix-browser-order.js Outdated Show resolved Hide resolved
scripts/fix-browser-order.js Outdated Show resolved Hide resolved
scripts/fix-feature-order.js Show resolved Hide resolved
scripts/migrations/002-remove-webview-flags.js Outdated Show resolved Hide resolved
scripts/migrations/002-remove-webview-flags.js Outdated Show resolved Hide resolved
test/linter/test-consistency.js Outdated Show resolved Hide resolved
test/linter/test-consistency.js Outdated Show resolved Hide resolved
test/linter/test-versions.js Outdated Show resolved Hide resolved
test/linter/test-versions.js Outdated Show resolved Hide resolved
test/linter/test-versions.js Outdated Show resolved Hide resolved
queengooborg and others added 2 commits May 12, 2022 13:29
@queengooborg @caugner
Apply suggestions from code review
53c2a5c 
Co-authored-by: Claas Augner <github@caugner.de>
@queengooborg
Address review comments
f46024f
@caugner caugner self-requested a review May 13, 2022 13:07
caugner
caugner requested changes May 13, 2022
View reviewed changes
Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

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

Few more nits, including one wrong type, but otherwise still looking good. 🚀

scripts/compare-features.js Outdated Show resolved Hide resolved
scripts/traverse.js Outdated Show resolved Hide resolved
scripts/compare-features.js Outdated Show resolved Hide resolved
scripts/statistics.js Outdated Show resolved Hide resolved
test/linter/test-browsers-presence.js Outdated Show resolved Hide resolved
@github-actions
Copy link

This pull request has merge conflicts that must be resolved before we can merge this.

@queengooborg @caugner
Apply suggestions from code review
04eda86 
Co-authored-by: Claas Augner <github@caugner.de>
@queengooborg queengooborg enabled auto-merge (squash) May 13, 2022 19:52
@queengooborg queengooborg merged commit 63dad76 into mdn:main May 13, 2022
@queengooborg queengooborg deleted the refactor/jsdocs branch May 13, 2022 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@caugner caugner caugner requested changes

Assignees
No one assigned
Labels
bulk_update An update to a mass amount of data, or scripts/linters related to such changes linter Issues or pull requests regarding the tests / linter of the JSON files. scripts Issues or pull requests regarding the scripts in scripts/.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@queengooborg @caugner