-
-
Notifications
You must be signed in to change notification settings - Fork 22
fix: re-enable noParamaterReAssign and fix linting #492
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
base: master
Are you sure you want to change the base?
fix: re-enable noParamaterReAssign and fix linting #492
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.
Please make sure to reflect this change only for functions where parameters are really reassigned within function.
| } | ||
|
|
||
| setUintBigint(uintBytes: number, offsetBytes: number, valueBN: bigint): void { | ||
| setUintBigint(uintBytes: number, offsetBytes: number, _valueBN: bigint): void { |
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 don't see valueBN is been reassigned anywhere in this function. Also primitives as parameters is not an issue to be assigned anyway.
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.
Hi there @nazarhussain . thanks for your comment. Actually it is being reassigned here
in the last else of the function.
also this pr tackles this issue. so this is the motivation behind these changes
Lines 19 to 20 in 1a34fe8
| // TODO: There are two many usages, will fix in separate PR | |
| "noParameterAssign": "off", |
re-enabling this biome flag requires these locations (i.e this prs changes) to be fixed. however i made sure that the the changes are compatabile with the current test suiting and dont change expected code functionality
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.
The changes related to primitives should be removed. All values of number, bigint or any other primitive is passed by value, so no issue if these are reassigned in local function scope.
We should focus the changes only object or complex types where accidental parameter change have any side effect.
Motivation
This PR takes care of a pending task, which is to re-enable the biome
noParamaterReAssignstyle lint and to fix all instances in the codebase where this rule is breachedchanges
noParamaterReAssign