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

Update dox to latest version and fix some documentation issues #12024

Merged
merged 46 commits into from Jul 8, 2022

Conversation

hasezoey
Copy link
Collaborator

@hasezoey hasezoey commented Jun 30, 2022

Summary

This PR updates dependency dox from 0.3.1(from 2015) to 0.9.1(from 2022), which should address #8925 at least a little

re #8925


This PR:

  • upgrades dox through the versions, a commit made each time a "minor" version was updated or when a change was required.
  • does not guarantee that all anchors or stuff are the same, i had modified some things to be more consistent so some could have changed.
  • should make debugging easier when dox fails with a error (5de8b9a)
  • adds yarn.lock got gitignore (because package-lock.jsonwas already in it)
  • fixes empty @returns tags to be properly set (meaning to have types)
  • fixes wrong array usage (things like [Type] were used when jsdoc only allows Type[] or Array<Type>)
  • removes function definitions from VirtualType.prototype.set and VirtualType.prototype.get because they were not proper jsdoc (also dox did not want to correctly register things like @callback or @typedef or @type with @name) (7155089)
  • adds many @memberOf tags to many jsdoc comments (because they were missing and resulting in wrong documentation)
  • fixes SchemaType.prototype.castFunction being listed as cast
  • removes @static and @function from SchemaType.prototype.castFunction
  • moves the jsdoc for DocumentArrayPath.set and SubdocumentPath.set to the correct location
  • replaces all (that i found) occurrences of - in @param descriptions, because they were making the documentation formatting weird (e57594c)
  • replaces all found occurrences of @param {*} with @param {Any} to be more consistent and also because * was making the documentation formatting weird (953853f)
  • removes all text after @static because in later versions of dox nothing was given to the processing anymore and was also not proper jsdoc (but things are still defined thanks to other things) (1719bf6)
  • adds a new property to ctx in docs/source/api.js named isFunction that after processing adds () to the end of the ctx.string, this was done because things like @function and @static could be in a different sequence than expected
  • changes to use ctx.constructor over data.name for static properties (to be consistent with non-static properties) (cfe7248)
  • removes a debug log of the ctx (39a02a1)

PS: this PR also fixes some documentation issues that are visible in current master

Note: please only someone with knowledge of how the documentation works merge this PR

This change is required because since dox 0.5.0 a error will be thrown if "return" present but empty
This change also extends the message to say what gets returned
This change is required because since dox 0.5.0 a error will be thrown if "return" present but empty
This change also extends the message to say what gets returned
…Type.set"

This is because dox (jsdoctypeparser@1.2.0) does not seem to recognize "@callback" or "@typedef" with "@name"
… "QueryCursor.prototype[Symbol.asyncIterator]"
This also changes the description for the parameters to match "SchemaType.set" and not cause bad formatting
Also upates the return type to "void"
This also sometimes somehow messes-up the documentation styling
The things that were defined there are set via other things anyway, and many did not even have it set
This also changes the description for the parameters to match "SubdocumentPath.set" and not cause bad formatting
Also upates the return type to "void"
…unction" and "static"

This fixes static "Schematype" properties to be in a different case than instance properties
@hasezoey
Copy link
Collaborator Author

hasezoey commented Jul 2, 2022

Note: there are currently conflicts with the base branch, but i will resolve them once it is ready for merge (after having had reviews)

lib/connection.js Outdated Show resolved Hide resolved
Copy link
Collaborator Author

@hasezoey hasezoey left a comment

Choose a reason for hiding this comment

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

I have now updated the branch to include the latest master again (because documentation changes were made), and i also have touched-up some of the documentation again, which includes:

  • adding some comments to signal what is doing what
  • move most string manipulation handling to after the "tags" loop
  • add a jsdoc "typedef" to remember all properties
  • add a proper type for @returns {void} or {undefined} as to not result in empty fields

Also some other discussion questions:

  • should tags and handling for @receiver be removed and be replaced with the proper tags everywhere?
  • should {Any} maybe be replaced again everywhere with {*}? because by jsdoc definition {*} is the proper "any" type, but currently otherwise causing some documentation style problems

Comment on lines +209 to +211
case 'deprecated':
ctx.deprecated = true;
break;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i have now included basic handling for @deprecated tag, but i am not sure where or in what style it should appear in the documentation

docs/source/api.js Show resolved Hide resolved
@hasezoey
Copy link
Collaborator Author

hasezoey commented Jul 4, 2022

Made another small update which does:

  • fix Aggregate.prototype.lookup not having a proper next line for a jsdoc tag
  • re-adding null to listed types when nullable
  • fixing Query.prototype.box jsdoc to be proper jsdoc and extending the comment for the parameters (please check if correct, i am not familiar with that function)
  • fixing @type tags not being properly escaped with { and }

Edit:
some other following changes:

  • add : to the end of all jsdoc headers that have #### to make them consistent
  • change parsed @return tag descriptions to overwrite property description instead of using a new property return, this was made because property return was never used and i am not familiar enough with pug
  • also changed the second @return parsing replace to allow for \n at the end (which seems to be common now)

…stead of adding to "return"

This is changed because "return" property was never used, but the "description" was (in pug files)
but i am not familiar with pug to change that
Also updates the second "replace" call because "\n" is mostly added to the end which makes it ineffective
This is to change the style to be consistent and headers with ":" seem to be the most used, so it was changed to that
@hasezoey
Copy link
Collaborator Author

hasezoey commented Jul 4, 2022

I would say this PR is finished for changes, unless some are requested / merging latest master, some changes could probably be put in their own PR / cherry-picked to land sooner.


i would like to request @vkarpov15 to review this PR for correctness

Copy link
Collaborator

@vkarpov15 vkarpov15 left a comment

Choose a reason for hiding this comment

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

Code is LGTM. I'll review this more after I merge, it's easier to test after merging to master because we don't have deployment previews.

@vkarpov15 vkarpov15 added this to the 6.4.4 milestone Jul 8, 2022
@vkarpov15 vkarpov15 merged commit d80a2ee into Automattic:master Jul 8, 2022
vkarpov15 added a commit that referenced this pull request Jul 8, 2022
@hasezoey
Copy link
Collaborator Author

hasezoey commented Jul 8, 2022

we don't have deployment previews.

isnt the script docs:view a deployment preview or do you maybe mean compiled documentation diff?

PS: also there were still some open discussion questions

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.

None yet

3 participants