Remove unused constants - Closes #1732 #1768
Conversation
|
Why comment them instead of remove? |
MaciejBaj
changed the title
|
@4miners I am gonna remove it as well, the PR is still in pending, I just wanted to build on jenkins to see if any errors i could find before rising a PR, but I will remove the unused constants and give a PR. |
|
@ManuGowda you could do it as well also on a branch or what's even better locally. Open PRs when they are ready. |
| @@ -118,8 +98,6 @@ var constants = { | |||
| // Testnet | |||
| 'da3ed6a45429278bac2666961289ca17ad86595d33b31037615d4b8e8f158bba', | |||
| ], | |||
| numberLength: 100000000, | |||
There was a problem hiding this comment.
numberLength should be renamed, kept and used along our test suites. We already have the same concept named normalizer instead of getting it from constants. I suggest to rename and get it in the test suite from here https://github.com/LiskHQ/lisk/blob/02d35b04afb2384db1e1b4257b0b8d202ea3fda5/test/common/utils/normalizer.js#L17
| @@ -83,17 +70,10 @@ var constants = { | |||
| dappDeposit: 10000000, | |||
| data: 10000000, | |||
| }, | |||
| feeStart: 1, | |||
| feeStartVolume: 10000 * 100000000, | |||
| fixedPoint: Math.pow(10, 8), | |||
There was a problem hiding this comment.
fixedPoint was used and should be used in here: https://github.com/LiskHQ/lisk/blob/02d35b04afb2384db1e1b4257b0b8d202ea3fda5/schema/swagger.yml#L1102 Also lisk-js uses it in their test suites as a hardcoded value. We need to ping @Tosch110 @shuse2 & @willclarktech once it is settled, suggesting to integrate it as a constant.
ManuGowda
force-pushed
the
1732-Remove_unused_helpers_constants_variables
branch
from
440a2a5
to
f8f9608
Compare
| @@ -118,8 +99,7 @@ var constants = { | |||
| // Testnet | |||
| 'da3ed6a45429278bac2666961289ca17ad86595d33b31037615d4b8e8f158bba', | |||
| ], | |||
| numberLength: 100000000, | |||
| requestLength: 104, | |||
| normalizer: 100000000, | |||
There was a problem hiding this comment.
normalizer means nothing, please use better name.
There was a problem hiding this comment.
normalizer is ok to me. It was like that before
MaciejBaj
dismissed
4miners’s stale review
normalizer was the name that was used before. Let's leave it for now as it is.
|
@MaciejBaj In tests, now we added it to |
What was the problem?
Unused constants in Lisk-core
How did I fix it?
Identified the usage of constants in Lisk-core and removed unused constants.
How to test it?
Review checklist