fix(): otp number no accurate #7

Closed

geraldlee2001 wants to merge 4 commits from fix/otp-number-no-accurate into master

geraldlee2001 commented

2021-10-27 04:19:01 -04:00

(Migrated from github.com)

fix for loop incorrect condition

codecov[bot] commented

2021-10-27 04:20:18 -04:00

(Migrated from github.com)

Codecov Report

Merging #7 (c3fda0b) into master (30fbba0) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master        #7   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           37        37           
  Branches         6         6           
=========================================
  Hits            37        37

Impacted Files	Coverage Δ
src/2FA.ts	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 30fbba0...c3fda0b. Read the comment docs.

# [Codecov](https://codecov.io/gh/boranseckin/2fa-utils/pull/7?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Boran+Seckin) Report > Merging [#7](https://codecov.io/gh/boranseckin/2fa-utils/pull/7?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Boran+Seckin) (c3fda0b) into [master](https://codecov.io/gh/boranseckin/2fa-utils/commit/30fbba0f6126e17e2b2faf05dc0a47e13bb60339?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Boran+Seckin) (30fbba0) will **not change** coverage. > The diff coverage is `100.00%`. [![Impacted file tree graph](https://codecov.io/gh/boranseckin/2fa-utils/pull/7/graphs/tree.svg?width=650&height=150&src=pr&token=k4uZcUNPCj&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Boran+Seckin)](https://codecov.io/gh/boranseckin/2fa-utils/pull/7?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Boran+Seckin) ```diff @@ Coverage Diff @@ ## master #7 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 1 1 Lines 37 37 Branches 6 6 ========================================= Hits 37 37 ``` | [Impacted Files](https://codecov.io/gh/boranseckin/2fa-utils/pull/7?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Boran+Seckin) | Coverage Δ | | |---|---|---| | [src/2FA.ts](https://codecov.io/gh/boranseckin/2fa-utils/pull/7/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Boran+Seckin#diff-c3JjLzJGQS50cw==) | `100.00% <100.00%> (ø)` | | ------ [Continue to review full report at Codecov](https://codecov.io/gh/boranseckin/2fa-utils/pull/7?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Boran+Seckin). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Boran+Seckin) > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/boranseckin/2fa-utils/pull/7?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Boran+Seckin). Last update [30fbba0...c3fda0b](https://codecov.io/gh/boranseckin/2fa-utils/pull/7?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Boran+Seckin). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Boran+Seckin).

boranseckin commented

2021-11-06 09:52:45 -04:00

(Migrated from github.com)

Thank you for contributing to this project @geraldlee2001. To observe the problem, I have written a better test that checks the function with random variables 10000 times (f2b0364805). And I was able to confirm that your solution works. To fix the problem, however, I will instead use the built-in padStart function (f6379e80f1). I think it will be more obvious and straightforward.

As a side note, it would be more beneficial if you had explained the problem and your suggested fixes better when opening a pull request. Also, the conflicting commits did not help when trying to understand the changes.

Thank you for contributing to this project @geraldlee2001. To observe the problem, I have written a better test that checks the function with random variables 10000 times (f2b0364805a46aae78e095c27ad1fb778ba5de93). And I was able to confirm that your solution works. To fix the problem, however, I will instead use the built-in `padStart` function (f6379e80f1da3cf5e905ec152cc2792235f97b39). I think it will be more obvious and straightforward. As a side note, it would be more beneficial if you had explained the problem and your suggested fixes better when opening a pull request. Also, the conflicting commits did not help when trying to understand the changes.

geraldlee2001 commented

2021-11-07 04:09:11 -05:00

(Migrated from github.com)

Thank you for contributing to this project @geraldlee2001. To observe the problem, I have written a better test that checks the function with random variables 10000 times (f2b0364). And I was able to confirm that your solution works. To fix the problem, however, I will instead use the built-in padStart function (f6379e8). I think it will be more obvious and straightforward.

As a side note, it would be more beneficial if you had explained the problem and your suggested fixes better when opening a pull request. Also, the conflicting commits did not help when trying to understand the changes.

i found that the otp digit some times with come out inaccurate requested amount. For example, I put 6 digits to the function but sometime will appear 5 digits only with a very rare chance after checking is the issue from library. After I changed to this code and the issue doesn't appear again

> Thank you for contributing to this project @geraldlee2001. To observe the problem, I have written a better test that checks the function with random variables 10000 times ([f2b0364](https://github.com/boranseckin/2fa-utils/commit/f2b0364805a46aae78e095c27ad1fb778ba5de93)). And I was able to confirm that your solution works. To fix the problem, however, I will instead use the built-in `padStart` function ([f6379e8](https://github.com/boranseckin/2fa-utils/commit/f6379e80f1da3cf5e905ec152cc2792235f97b39)). I think it will be more obvious and straightforward. > > As a side note, it would be more beneficial if you had explained the problem and your suggested fixes better when opening a pull request. Also, the conflicting commits did not help when trying to understand the changes. i found that the otp digit some times with come out inaccurate requested amount. For example, I put 6 digits to the function but sometime will appear 5 digits only with a very rare chance after checking is the issue from library. After I changed to this code and the issue doesn't appear again

👍 1