Skip to content
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

chore: now bn accepts null #523

Merged
merged 2 commits into from
Sep 27, 2022
Merged

chore: now bn accepts null #523

merged 2 commits into from
Sep 27, 2022

Conversation

LuizAsFight
Copy link
Contributor

No description provided.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 27, 2022

Coverage report

St.
Category Percentage Covered / Total
🟢 Statements 89.2% 3443/3860
🟡 Branches 70.19% 657/936
🟢 Functions 86.54% 688/795
🟢 Lines 89.13% 3296/3698

Test suite run success

520 tests passing in 46 suites.

Report generated by 🧪jest coverage report action from cdbafcd

@LuizAsFight LuizAsFight marked this pull request as ready for review September 27, 2022 04:22
@LuizAsFight LuizAsFight enabled auto-merge (squash) September 27, 2022 06:06
@@ -251,7 +251,7 @@ export class BN extends BnJs implements BNInputOverrides, BNHiddenTypes, BNHelpe
}

// functional shortcut to create BN
export const bn = (value?: BNInput, base?: number | 'hex', endian?: BnJs.Endianness) =>
export const bn = (value?: BNInput | null, base?: number | 'hex', endian?: BnJs.Endianness) =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we change the type BNInput?

Copy link
Contributor Author

@LuizAsFight LuizAsFight Sep 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think no. a lot of helper methods use BNInput but shouldn't accept a null/undefined as values.

for example:
bn(10).mul() should not work. It's needed to explicitly say the value I want to multiply to mul method, which has BNInput as input typing

@LuizAsFight LuizAsFight merged commit f106a78 into master Sep 27, 2022
@LuizAsFight LuizAsFight deleted the lf/feat/math-bn-accept-null branch September 27, 2022 13:29
@LuizAsFight LuizAsFight self-assigned this Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants