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

add BigInt.fromString and BigInt.fromNumber #2399

Merged
merged 11 commits into from
Mar 25, 2024

Conversation

coleea
Copy link
Contributor

@coleea coleea commented Mar 24, 2024

Type

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Description

According to #1766 , I added 4 functions.

Number.fromString
Number.fromBigInt
BigInt.fromString
BigInt.fromNumber

I changed number variable in Number.ts to number_ for using Number.MAX_SAFE_INTEGER and Number.MIN_SAFE_INTEGER

Related

Copy link

changeset-bot bot commented Mar 24, 2024

🦋 Changeset detected

Latest commit: e946010

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages
Name Type
effect Patch
@effect/cli Patch
@effect/experimental Patch
@effect/opentelemetry Patch
@effect/platform-browser Patch
@effect/platform-bun Patch
@effect/platform-node-shared Patch
@effect/platform-node Patch
@effect/platform Patch
@effect/printer-ansi Patch
@effect/printer Patch
@effect/rpc-http Patch
@effect/rpc Patch
@effect/schema Patch
@effect/typeclass Patch
@effect/vitest Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot changed the base branch from main to next-major March 24, 2024 12:04
@coleea coleea changed the title feat. BigInt.fromString, BigInt.fromNumber, Number.fromBigInt, Number.fromString feat. add 4 functions : BigInt.fromString, BigInt.fromNumber, Number.fromBigInt, Number.fromString Mar 24, 2024
fubhy
fubhy previously requested changes Mar 24, 2024
packages/effect/src/BigInt.ts Outdated Show resolved Hide resolved
packages/effect/src/Number.ts Outdated Show resolved Hide resolved
packages/effect/src/Number.ts Outdated Show resolved Hide resolved
packages/effect/src/Number.ts Outdated Show resolved Hide resolved
packages/effect/src/BigInt.ts Outdated Show resolved Hide resolved
packages/effect/src/Number.ts Outdated Show resolved Hide resolved
packages/effect/src/Number.ts Outdated Show resolved Hide resolved
Comment on lines 504 to 507
* @param s - The string to be converted to a `number`.
* @returns An `Option` type that is either `some(number)` if the string can be converted
* to a valid number, or `none` if the string is empty or contains invalid characters
* for number conversion.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @param s - The string to be converted to a `number`.
* @returns An `Option` type that is either `some(number)` if the string can be converted
* to a valid number, or `none` if the string is empty or contains invalid characters
* for number conversion.
* @param s - The string to be converted to a `number`.

Redundant (repetition of function description)

packages/effect/src/BigInt.ts Outdated Show resolved Hide resolved
packages/effect/src/BigInt.ts Outdated Show resolved Hide resolved
@coleea
Copy link
Contributor Author

coleea commented Mar 24, 2024

@fubhy thank you. I applied your change-request

@coleea coleea requested a review from fubhy March 24, 2024 13:19
.changeset/sixty-news-begin.md Outdated Show resolved Hide resolved
packages/effect/src/Number.ts Outdated Show resolved Hide resolved
@coleea coleea requested a review from tim-smart March 24, 2024 23:32
@tim-smart tim-smart changed the base branch from next-major to main March 24, 2024 23:38
packages/effect/src/Number.ts Outdated Show resolved Hide resolved
packages/effect/test/Number.test.ts Outdated Show resolved Hide resolved
.changeset/sixty-news-begin.md Outdated Show resolved Hide resolved
@tim-smart tim-smart changed the title feat. add 4 functions : BigInt.fromString, BigInt.fromNumber, Number.fromBigInt, Number.fromString add BigInt.fromString and BigInt.fromNumber Mar 24, 2024
packages/effect/src/Number.ts Outdated Show resolved Hide resolved
Copy link
Member

@tim-smart tim-smart left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

@tim-smart tim-smart merged commit 54b7c00 into Effect-TS:main Mar 25, 2024
12 checks passed
@coleea
Copy link
Contributor Author

coleea commented Mar 25, 2024

@tim-smart @fubhy thank you for patience 👍

@github-actions github-actions bot mentioned this pull request Mar 25, 2024
@coleea coleea deleted the feature-add-functions branch March 29, 2024 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants