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

Docs Update: Improved Function and Class Descriptions #1895

Closed
wants to merge 2 commits into from

Conversation

coudron
Copy link

@coudron coudron commented Jun 20, 2023

This pull request offers concise yet comprehensive updates to function and class documentation.

This PR contains no functional changes, only documentation improvements.

@shadowspawn
Copy link
Collaborator

Has this been generated by OtterDoc?

There are some potentially useful changes, but also several places where a new JSDoc is added instead of updated, comments in the code hoisted into the JSDoc, and throw incorrectly described as being thrown.

@coudron
Copy link
Author

coudron commented Jun 20, 2023

@shadowspawn Yeah, these were generated using OtterDoc. We just started developing it and looking to get feedback and determine how useful people think it is.

I'm wondering if people consider this too verbose with its additions?

Yeah, it's hoisting some of the params up from the outer scope. That's probably not the desired way to document it.

The throws appeared correct to me? I appreciate it when docs tell me what is possibly thrown, but curious how others view it.

Feedback welcome. Hope this PR could be massaged into being helpful as it's not perfect on the first pass.

* Sets the allowed choices for the argument and returns the current instance of the class.
* @param {Array} values - An array of allowed choices.
* @returns {Object} - The current instance of the class.
* @throws {InvalidArgumentError} - If the argument is not one of the allowed choices.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This routine contains a throw in the code, but does not actually throw an error that can be caught when it is called.

  1. The throw is in a custom.parseArg function which is called later when the command-line arguments are being parsed.
  2. The error is actually caught during processing and repackaged.

@@ -26,6 +26,11 @@ class Help {
* @returns {Command[]}
*/

/**
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is already an existing JSDoc for this routine. This should presumably be a modification rather than an addition. (This is true of many of the additions.)

@shadowspawn
Copy link
Collaborator

I added a couple of comments to explain two of the issues I noticed.

I think the suggestions would be welcome while I was actually adding new code or working on a routine. But not as a change-lots PR.

In this project we have manually maintained dual documentation in the JavaScript files and the TypeScript interface definition files. I think there is too much work to take the generic suggested improvements in the JavaScript and fix up the problems, and reproduce the changes.

@coudron
Copy link
Author

coudron commented Jun 21, 2023

Understood. Appreciate the feedback.

This is a good use case to understand and make adjustments to OtterDoc. I think a lot of people feel the same way.

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