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

SNOW-922989 Replace deprecated bignumber.js floor function #649

Conversation

mblakley
Copy link
Contributor

Description

Replaced the deprecated floor function in new versions of bignumber.js (>= 6.0) with the updated solution of calling .integerValue(BigNumber.ROUND_FLOOR)

Checklist

  • Format code according to the existing code style (run npm run lint:check -- CHANGED_FILES and fix problems in changed code) - I ran this, and there are a lot of warnings in the existing column.js file.
  • Create tests which fail without the change (if possible)
  • Make all tests (unit and integration) pass (npm run test:unit and npm run test:integration) - Ran unit tests, no new failures
  • Extend the README / documentation and ensure is properly displayed (if necessary)
  • Provide JIRA issue id (if possible) or GitHub issue id in commit message

@mblakley mblakley requested a review from a team as a code owner September 25, 2023 14:24
@github-actions
Copy link

github-actions bot commented Sep 25, 2023

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@mblakley
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@mblakley
Copy link
Contributor Author

Thank you for reviewing! Another possible solution would be to set an upper bound on the bignumber.js version in package.json - my initial thought was that my proposed solution would allow moving to the newest versions of that package instead of sticking with the older versions.

@sfc-gh-pbulawa
Copy link
Collaborator

Hi @mblakley,

Thanks for your contribution! I have ran your branch in #656 and it seems that the newer version of the bignumber.js is having different API:

Uncaught TypeError: valFracSecsWithTzBig.greaterThanOrEqualTo is not a function
      at Object.convertRawTimestampTz [as convert] (/mnt/host/lib/connection/result/column.js:462:30)

Could you please take a look into it and update the code?

@mblakley
Copy link
Contributor Author

I made updates to my branch.

Luckily, the API name updates for greaterThanOrEqualTo were also done in v6.0 of the bignumber.js package, so there's no conflict with using that version as the minimum.

package.json Outdated Show resolved Hide resolved
@sfc-gh-pmotacki sfc-gh-pmotacki force-pushed the SNOW-922989_bignumber_deprecated_floor branch from dba848f to 9cc2af8 Compare October 9, 2023 10:21
@sfc-gh-pmotacki sfc-gh-pmotacki force-pushed the SNOW-922989_bignumber_deprecated_floor branch from cac224d to 06a7d40 Compare October 13, 2023 13:41
@sfc-gh-pmotacki
Copy link
Collaborator

@mblakley thank you for your contribution.

@sfc-gh-pmotacki sfc-gh-pmotacki merged commit bcdcd99 into snowflakedb:master Oct 19, 2023
11 of 12 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Oct 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants