Skip to content

Improve precision of IDB API and add tests #569

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Sep 9, 2021
Merged

Improve precision of IDB API and add tests #569

merged 8 commits into from
Sep 9, 2021

Conversation

japgolly
Copy link
Contributor

@japgolly japgolly commented Sep 6, 2021

No description provided.

@japgolly japgolly requested a review from armanbilge September 6, 2021 06:04
@japgolly
Copy link
Contributor Author

japgolly commented Sep 6, 2021

wait up i'll rebase

@japgolly japgolly force-pushed the topic/idb branch 2 times, most recently from a25e5b7 to 1023e0c Compare September 6, 2021 06:14
@japgolly
Copy link
Contributor Author

japgolly commented Sep 6, 2021

Ok cool, rebased, tweaked, and ready for review

@japgolly japgolly added this to the v2.0.0 milestone Sep 6, 2021
armanbilge
armanbilge reviewed Sep 7, 2021
View reviewed changes
Copy link
Member

@armanbilge armanbilge left a comment

Choose a reason for hiding this comment

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

This looks pretty awesome, adding type parameters to this API is obviously a big win. Most just questions since I'm not so familiar with IDB.

@japgolly
Replace use of js.Any in the IDB API with Any
e0d2f58
@japgolly
IDB counts should be Ints, not Doubles.
a731b36 
Acording to
https://developer.mozilla.org/en-US/docs/Web/API/IDBObjectStore/getAll#parameters
the max count is 2^32
@japgolly
More improvement
7a8708e 
* Stop using js.Any in IDB
* Replace all uses of Any with something more specific
* Improve type safety of various methods
* Add missing API
* Add more ScalaDoc
* Change constants to match the JS names
@japgolly
Clarify type parameter meanings in type aliases
af0c635
@japgolly
Fix ScalaDoc
d3dbba7
@japgolly
Bugfix, thanks CI
25e7783
@japgolly japgolly requested a review from armanbilge September 9, 2021 00:25
@japgolly
Copy link
Contributor Author

japgolly commented Sep 9, 2021

Ok @armanbilge I've updated this again, and addressed all the feedback. You can read the expanded commit messages for the changes.

armanbilge
armanbilge approved these changes Sep 9, 2021
View reviewed changes
Copy link
Member

@armanbilge armanbilge 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 your massive efforts on this!! Learned a lot too.

@armanbilge armanbilge merged commit bece5e6 into master Sep 9, 2021
@armanbilge armanbilge deleted the topic/idb branch September 9, 2021 01:05
@japgolly
Copy link
Contributor Author

japgolly commented Sep 9, 2021

Awesome! Thanks for the lengthy review too

@armanbilge armanbilge mentioned this pull request Oct 30, 2021
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sjrd sjrd sjrd left review comments

@armanbilge armanbilge armanbilge approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@japgolly @sjrd @armanbilge