-
Notifications
You must be signed in to change notification settings - Fork 72
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
Improve 'no-identical-functions': add option for min function size #325
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me overall, but there is one thing we need to investigate before I can approve.
src/rules/no-identical-functions.ts
Outdated
anyOf: [ | ||
{ type: 'integer', minimum: 3 }, | ||
{ | ||
enum: ['sonar-runtime'], | ||
}, | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure, but we need to test locally if this anyOf
property will still work with the following logic in SonarJS:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
definitely it will not:) good catch. I will see what I can do
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reverted the change with anyOf
. We will have to update configuration of the rule in SonarJS.
I don't see other way :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
src/rules/no-identical-functions.ts
Outdated
enum: ['sonar-runtime'], | ||
}, | ||
], | ||
schema: [OPTION_SCHEMA, OPTION_SCHEMA], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you figure out why we need it twice ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, each repetition is a choice between two options so that you can provide them in any order
Kudos, SonarCloud Quality Gate passed! |
Fixes #225