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

Modernize sdk #620

Merged
merged 6 commits into from
Aug 4, 2018
Merged

Modernize sdk #620

merged 6 commits into from
Aug 4, 2018

Conversation

flovilmart
Copy link
Contributor

  • Removes ParsePromise
  • Removes backbone style options
  • Updates documentations.

- Replace ParsePromise with native Promise
- Removes all compatibility with Backbone style callbacks
@codecov
Copy link

codecov bot commented Aug 2, 2018

Codecov Report

Merging #620 into master will increase coverage by 0.02%.
The diff coverage is 76.47%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #620      +/-   ##
=========================================
+ Coverage   84.57%   84.6%   +0.02%     
=========================================
  Files          48      48              
  Lines        4039    3851     -188     
  Branches      911     871      -40     
=========================================
- Hits         3416    3258     -158     
+ Misses        623     593      -30
Impacted Files Coverage Δ
src/ObjectStateMutations.js 100% <ø> (ø) ⬆️
src/FacebookUtils.js 5.55% <ø> (ø) ⬆️
src/ParseSchema.js 100% <ø> (ø) ⬆️
src/CoreManager.js 100% <ø> (ø) ⬆️
src/StorageController.browser.js 100% <ø> (ø) ⬆️
src/ParseSession.js 89.74% <0%> (ø) ⬆️
src/LiveQuerySubscription.js 50% <0%> (+4.54%) ⬆️
src/ParseGeoPoint.js 95.31% <0%> (+5.6%) ⬆️
src/ParseHooks.js 92.06% <0%> (ø) ⬆️
src/Push.js 100% <100%> (ø) ⬆️
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b762756...3c4efb9. Read the comment docs.

@flovilmart flovilmart requested a review from dplewis August 2, 2018 19:59
Copy link
Member

@dplewis dplewis 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, although I don’t know much about jest.runAllTicks(), flushingPromises(), setImmediate and the such

@flovilmart
Copy link
Contributor Author

The problem was that some executions needed to wait till all current blocks pending in the promises should be run. That changes with using JS promises. The Parse Promises would run the continuation blocks immediately if possible. With JS, those blocks are enqueue, therefore, we needed a way to flush the current blocks :)

@dplewis
Copy link
Member

dplewis commented Aug 3, 2018

Ah makes sense, I learned something new.

@flovilmart
Copy link
Contributor Author

Had to learn it the hard way!

@flovilmart flovilmart merged commit c344fd4 into master Aug 4, 2018
@flovilmart flovilmart deleted the modernize-sdk branch August 4, 2018 18:24
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