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

1209 feature add jsdoc linter #1210

Open
wants to merge 35 commits into
base: devel
Choose a base branch
from

Conversation

JoshuaSBrown
Copy link
Collaborator

@JoshuaSBrown JoshuaSBrown commented Jan 7, 2025

PR Description

Adding jsdoc linter will help to keep code changes consistent between developers and limit the intellectual overhead of seeing white space changes in the source code. This PR fixes the problem by using a formatter and a linter to help with code quality.

Tasks

  • - A description of the PR has been provided, and a diagram included if it is a new feature.
  • - Formatter has been run
  • - Labels have been assigned to the pr
  • - A reviwer has been added
  • - A user has been assigned to work on the pr

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

We failed to fetch the diff for pull request #1210

You can try again by commenting this pull request with @sourcery-ai review, or contact us for help.

@JoshuaSBrown JoshuaSBrown changed the base branch from master to devel January 7, 2025 12:41
@JoshuaSBrown JoshuaSBrown self-assigned this Jan 7, 2025
@JoshuaSBrown JoshuaSBrown added Type: Refactor Imlplementation change, same functionality Component: CI labels Jan 7, 2025
@JoshuaSBrown JoshuaSBrown linked an issue Jan 7, 2025 that may be closed by this pull request
Copy link
Collaborator

@t-ramz t-ramz left a comment

Choose a reason for hiding this comment

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

I made a few comments.

I believe the examples of endpoint usage are maybe misleading.

I'm also unsure about nested object attributes being marked as explicit params.

core/database/foxx/api/data_router.js Outdated Show resolved Hide resolved
core/database/foxx/api/data_router.js Show resolved Hide resolved
core/database/foxx/api/process.js Show resolved Hide resolved
core/database/foxx/api/repo_router.js Outdated Show resolved Hide resolved
eslint.config.js Outdated Show resolved Hide resolved
web/static/jquery/jquery.js Show resolved Hide resolved
web/static/jquery/jquery.js Show resolved Hide resolved
Copy link
Collaborator

@AronPerez AronPerez left a comment

Choose a reason for hiding this comment

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

Some questions on example docs and specified files

core/database/foxx/api/task_router.js Outdated Show resolved Hide resolved
eslint.config.js Outdated Show resolved Hide resolved
eslint.config.js Outdated Show resolved Hide resolved
Remove nested docs.
* output: ['usr', 'local', 'bin', 'node']
* @throws {Error} If the provided path is not a valid string.
*
* @example
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this example show usage as well? e.g. const splitPath = splitPOSIXPath(posixPath);


module.exports = [
{
ignores: ["docs/_static/**/*", "web/static/jquery/jquery.js", "web/node_modules"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will this also run on build for the Foxx API?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It runs for everything in the repo? Are you worried about a node_modules file in the foxx api? I don't think that will happen because foxx installs in the database not locally on the filesystem.

@@ -3188,9 +3189,12 @@ module.exports = (function () {
};

/**
* Parses other_token_data string field and breaks it into pieces
*
* @param {integer} token_type - Type to determine parse logic.
Copy link
Collaborator

Choose a reason for hiding this comment

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

It didn't like my typing? 😢

Copy link
Collaborator

@t-ramz t-ramz left a comment

Choose a reason for hiding this comment

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

Looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: CI Type: Refactor Imlplementation change, same functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[JSDoc] - Apply JSDoc linter across code base
3 participants