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

descending parameter should be optional in JS wrapper #317

Open
scottexton opened this issue Oct 22, 2024 · 7 comments
Open

descending parameter should be optional in JS wrapper #317

scottexton opened this issue Oct 22, 2024 · 7 comments

Comments

@scottexton
Copy link

No description provided.

@scottexton
Copy link
Author

A recent commit changed the Store.scan and Session.fetchAll JavaScript APIs so that the new 'descending' field is now required. Was this planned, or by mistake? Shouldn't we just have a default like there is for the other options.

@swcurran
Copy link
Contributor

@ff137 — I’m guessing that was a change you made? ^^^^

@ff137
Copy link
Contributor

ff137 commented Oct 22, 2024

@scottexton the relevant changes were made here: https://github.com/hyperledger/aries-askar/pull/291/files

My intention was for the new fields to be optional / have defaults, so that there are no breaking changes. I think I managed that in the rust/python code, but not in the JavaScript wrapper... apologies.

Looks like descending: boolean should be descending?: boolean in the wrappers/javascript/packages/aries-askar-shared methods.

@scottexton
Copy link
Author

@ff137 Will you make a fix for that, or do you want me to?

@ff137
Copy link
Contributor

ff137 commented Oct 22, 2024

@scottexton I do feel responsible to clean up my mess and make a PR, but review-approve takes time. It's probably best if you can test a fix, given that you're working with the js wrapper.

Should just involve adding a ? to those descending: bool lines

@scottexton
Copy link
Author

@ff137 Alright. I will include a fix in amongst my other changes.

@scottexton
Copy link
Author

A pull request has been created with a fix for this issue (#319) - this fix is embedded within a larger change to introduce ODBC backend support.

@andrewwhitehead andrewwhitehead changed the title 'descending descending parameter should be optional in JS wrapper Nov 25, 2024
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

No branches or pull requests

3 participants