search

smartcar / node-sdk

Smartcar Node.js SDK
MIT License
49 stars 14 forks source link

BREAKING CHANGE: Update and streamline SDKs #132

Closed s-ashwinkumar closed 3 years ago

s-ashwinkumar commented 3 years ago

Asana: https://app.asana.com/0/1200041805723615/1200544489692892/f

codecov[bot] commented 3 years ago

Codecov Report

Merging #132 (8555ed2) into master (ad46f07) will not change coverage. The diff coverage is 100.00%.

:exclamation: Current head 8555ed2 differs from pull request most recent head be95edf. Consider uploading reports for the commit be95edf to get more accurate results Impacted file tree graph

@@            Coverage Diff            @@
##            master      #132   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         7    +2     
  Lines          303       220   -83     
=========================================
- Hits           303       220   -83
Impacted Files Coverage Δ
index.js 100.00% <100.00%> (ø)
lib/auth-client.js 100.00% <100.00%> (ø)
lib/smartcar-error.js 100.00% <100.00%> (ø)
lib/smartcar-service.js 100.00% <100.00%> (ø)
lib/util.js 100.00% <100.00%> (ø)
lib/vehicle.js 100.00% <100.00%> (ø)
.eslintrc.js 100.00% <0.00%> (ø)

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 ad46f07...be95edf. Read the comment docs.

nbry commented 3 years ago

I did a review of the SDK, and it looks pretty good to me! Your method map is really cool.

Let's wait for @rsimari 's review and see what he thinks.

nbry commented 3 years ago

Committed a change to smartcar-service.js, that allows for a better naming when a user sends a batch request that includes a request to vehicle attributes. In other words, the batch response for attributes will be batch.attributes() instead of batch['']()

nbry commented 3 years ago

Review 2: @s-ashwinkumar , I did another run-through of the node-sdk. There was a change on Friday to the Smartcar Connect flow, where upon choosing permissions for your car, a "success" page is displayed. However, now you have to press 'continue' to get the url with the access code in the params. For clarity, the continue button has an id of continue-button