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

Fix annotation of VFS search function #70

Merged
merged 2 commits into from
Nov 7, 2022

Conversation

mahsashadi
Copy link
Contributor

It seems search function does not return boolean.

Copy link
Member

@ajmeese7 ajmeese7 left a comment

Choose a reason for hiding this comment

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

Looks good to me, it might benefit by having an actual type for the array instead of just saying that it's an array of objects, but that's up to @andersevenrud. Good catch 👍🏼

@andersevenrud
Copy link
Member

having an actual type

That would be preferable. I checked, and this package don't have many of them documented.

Also, a lot of the annotations in this file is invalid. Should actually be Promise<T> for pretty much all of them 😅

@andersevenrud
Copy link
Member

A good commit message for this would be: docs(vfs): update search function annotation.

This repo uses https://www.conventionalcommits.org/en/v1.0.0/

@mahsashadi
Copy link
Contributor Author

A good commit message for this would be: docs(vfs): update search function annotation.

This repo uses https://www.conventionalcommits.org/en/v1.0.0/

Done
@andersevenrud

@andersevenrud
Copy link
Member

Done

Could you make it a Promise<Object> ?

@andersevenrud andersevenrud merged commit c217913 into os-js:master Nov 7, 2022
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