Skip to content

Conversation

@NmN03jain
Copy link

Thank you for the time you are putting into AlaSQL!

Copy link
Member

@mathiasrw mathiasrw left a comment

Choose a reason for hiding this comment

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

Please add a test file that verifies the bahavour so we dont accidentally break it at some point.

Copy the test/test000.js to test/test1455.js and adapt the logic

@NmN03jain
Copy link
Author

@mathiasrw requested chages are made ,can you pls verify .

@NmN03jain
Copy link
Author

@mathiasrw what changes i need to performe ?

@NmN03jain NmN03jain requested a review from mathiasrw August 15, 2023 16:04
@mathiasrw
Copy link
Member

image

Please insert a .toString() at the end of the value and make sure we dont throw an error.

@mathiasrw mathiasrw mentioned this pull request Aug 17, 2023
@NmN03jain
Copy link
Author

@mathiasrw any more changes required ?

@mathiasrw
Copy link
Member

Yes. Please see the pending review changes

image image image

@mathiasrw
Copy link
Member

Please make sure that npx yarn test is working locally before pushing.

@mathiasrw
Copy link
Member

How can I help get the last little bits aligned?

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.

2 participants