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

refactor: json config structure #42

Merged
merged 4 commits into from
Feb 22, 2024
Merged

refactor: json config structure #42

merged 4 commits into from
Feb 22, 2024

Conversation

excaliborr
Copy link

🤖 Linear

Doesn't fully complete the linear task

  • Adds ability to use a JSON config file instead of just CLI
  • Does not implement the new config parameters logic

@excaliborr excaliborr changed the title refactor: config structure refactor: json config structure Feb 21, 2024
package.json Outdated
"@types/jest": "29.5.11",
"@types/node": "20.10.7",
"@typescript-eslint/parser": "6.2.0",
"ajv": "8.12.0",
Copy link
Member

Choose a reason for hiding this comment

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

Should these 2 packages be in the dependencies section?

src/types.ts Outdated
notice: Type.Boolean(),
return: Type.Boolean(),
}),
inheritdoc: Type.Optional(Type.Boolean()),
Copy link
Member

Choose a reason for hiding this comment

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

I believe the inheritdoc option will either replace enforceInheritdoc or be under functions as it does not make sense to have inheritdoc enabled for external functions but not for public, or have it for functions but not for variables. It's ok as it is for now though, we will get to it when reading / using the config.

src/types.ts Show resolved Hide resolved
src/utils.ts Outdated
root: detectedConfig.root ?? './',
functions: detectedConfig.functions,
enforceInheritdoc: detectedConfig.enforceInheritdoc ?? true,
constructorNatspec: detectedConfig.constructorNatspec ?? false,
Copy link
Member

Choose a reason for hiding this comment

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

A TODO for one of the next PRs: deprecation logic will be defined here

@gas1cent gas1cent added this to the v1.2.0 milestone Feb 22, 2024
src/types.ts Outdated
Comment on lines 19 to 20
notice: Type.Boolean(),
return: Type.Boolean(),
Copy link
Member

@gas1cent gas1cent Feb 22, 2024

Choose a reason for hiding this comment

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

notice and return should be true by default

Copy link
Member

Choose a reason for hiding this comment

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

And add param perhaps?

});
});

it('Should revert with an invalid config', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

Lowercase to match the rest of the tests 😬

Suggested change
it('Should revert with an invalid config', async () => {
it('should revert with an invalid config', async () => {

package.json Outdated
Comment on lines 41 to 43
"@typescript-eslint/parser": "6.2.0",
"eslint": "8.56.0",
"eslint-plugin-jsdoc": "48.1.0",
Copy link
Member

Choose a reason for hiding this comment

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

Pls revert, these are useful packages

Copy link
Author

Choose a reason for hiding this comment

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

how did this even happen lmao

@gas1cent gas1cent merged commit 70bf763 into main Feb 22, 2024
4 checks passed
@gas1cent gas1cent deleted the feat/new-config branch February 22, 2024 14:48
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