Closed knoxcard closed 2 years ago
OMG FINALLY, i could have rewritten the entire NodeJS framework in that amount of time.
Greenkeeper = good bot Travis = crap bot
Turned 14 lines of code into 2 lines...good riddance
You removed validations (e.g. what if it's less than 0) and added forced casting to number. But the numbers need to be checked to be > 0 and Int.
@DiegoRBaquero - all checks passed, is that particular logic you mentioned not included in the test run?
I guess not, tests should be added for such scenarios and check that it fails.
gotchya, i will alter the code now...
@DiegoRBaquero
> 0
failed, appears that zero is inclusive, a valid option. > -1
passed the tests.
@DiegoRBaquero @knoxcard Need any help pushing this forward? I think it's a good idea, because everything set from ENV is a string by default, is see no harm in Number
/parseInt
and then check the other conditions.
Linking our related issue here - https://github.com/citizenos/citizenos-api/issues/137
@DiegoRBaquero Any plans on implementing this? I understand that probably there are many other things to do, but I need to know what's going to happen with this. If you have no plans of implementing this, then we'll just move on with other solutions. All the best!
Hello everyone, what is the use case for having sequelize-pool convert these configuration values to Number? I did not understand. Why can't the user enforce the correct types? Why would an user pass something that is not a number here?
automatically set pool.max and pool.min to integer, if they are defined and
> -1
, or else set safe defaults instead of throwing errors