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.

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

Feature/issue 552 - New vehicle data - FuelRange #601

Merged
merged 3 commits into from
Jun 27, 2018
Merged

Feature/issue 552 - New vehicle data - FuelRange #601

merged 3 commits into from
Jun 27, 2018

Conversation

mrapitis
Copy link
Contributor

Fixes #552

This PR is ready for review.

Risk

This PR makes minor API changes.

Testing Plan

Added new unit tests and expanded existing ones

Summary

Added Vehicle Data RPC's, structs, enums as defined in the fuel range proposal.

CLA

  • I have signed the CLA

@mrapitis
Copy link
Contributor Author

mrapitis commented Mar 6, 2018

@BrettyWhite @bilal-alsharifi please review when time permits. thanks!

@codecov-io
Copy link

codecov-io commented Mar 6, 2018 •
edited
Loading

Codecov Report

Merging #601 into develop will decrease coverage by 3.9%.
The diff coverage is 97.67%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop     #601      +/-   ##
=============================================
- Coverage      41.38%   37.48%   -3.91%     
- Complexity      2656     2693      +37     
=============================================
  Files            362      364       +2     
  Lines          15831    18163    +2332     
  Branches        1684     2310     +626     
=============================================
+ Hits            6552     6808     +256     
- Misses          8971    11031    +2060     
- Partials         308      324      +16
Impacted Files Coverage Δ Complexity Δ
...celink/proxy/rpc/SubscribeVehicleDataResponse.java 96.34% <100%> (+0.13%) 52 <2> (+2) ⬆️
...a/com/smartdevicelink/proxy/rpc/OnVehicleData.java 94.56% <100%> (+0.18%) 53 <2> (+2) ⬆️
...rtdevicelink/proxy/rpc/UnsubscribeVehicleData.java 98.78% <100%> (+0.04%) 53 <2> (+2) ⬆️
...martdevicelink/proxy/rpc/SubscribeVehicleData.java 98.78% <100%> (+0.04%) 53 <2> (+2) ⬆️
...artdevicelink/proxy/rpc/enums/VehicleDataType.java 100% <100%> (ø) 2 <0> (ø) ⬇️
...link/proxy/rpc/UnsubscribeVehicleDataResponse.java 96.34% <100%> (+0.13%) 52 <2> (+2) ⬆️
.../com/smartdevicelink/proxy/rpc/GetVehicleData.java 98.82% <100%> (+0.04%) 55 <2> (+2) ⬆️
.../java/com/smartdevicelink/proxy/rpc/FuelRange.java 100% <100%> (ø) 6 <6> (?)
.../com/smartdevicelink/proxy/rpc/enums/FuelType.java 100% <100%> (ø) 2 <2> (?)
...rtdevicelink/proxy/rpc/GetVehicleDataResponse.java 95.65% <66.66%> (-0.98%) 53 <1> (+1)
... and 9 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 54a9854...bb58ea2. Read the comment docs.

bilal-alsharifi
bilal-alsharifi reviewed Mar 10, 2018
View reviewed changes
Copy link
Contributor

@bilal-alsharifi bilal-alsharifi left a comment

Choose a reason for hiding this comment

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

Everything looks good except some extra spaces in some parts of the modified files.

.../src/androidTest/java/com/smartdevicelink/test/rpc/requests/UnsubscribeVehicleDataTests.java Outdated
@@ -91,7 +92,8 @@ protected JSONObject getExpectedParameters(int sdlVersion) {
result.put(UnsubscribeVehicleData.KEY_AIRBAG_STATUS, Test.GENERAL_BOOLEAN);
result.put(UnsubscribeVehicleData.KEY_EMERGENCY_EVENT, Test.GENERAL_BOOLEAN);
result.put(UnsubscribeVehicleData.KEY_CLUSTER_MODE_STATUS, Test.GENERAL_BOOLEAN);
result.put(UnsubscribeVehicleData.KEY_MY_KEY, Test.GENERAL_BOOLEAN);
result.put(UnsubscribeVehicleData.KEY_MY_KEY, Test.GENERAL_BOOLEAN);
result.put(UnsubscribeVehicleData.KEY_FUEL_RANGE, Test.GENERAL_BOOLEAN);
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like there are extra spaces/tabs here and in some other places in the modifications

@mrapitis
Copy link
Contributor Author

@bilal-alsharifi please re-review when time permits. thanks!

@bilal-alsharifi
Copy link
Contributor

Thanks, @mrapitis. There are still some minor spacing issues in the following files:

OnVehicleDataTests.java
GetVehicleDataTests.java
SubscribeVehicleDataTests.java
GetVehicleDataResponseTests.java
SubscribeVehicleDataResponseTest.java
GetVehicleData.java
GetVehicleDataResponse.java
OnVehicleData.java
SubscribeVehicleDataResponse.java
UnsubscribeVehicleDataResponse.java

@mrapitis
Copy link
Contributor Author

@bilal-alsharifi thanks for the review, please check once more when available.

@bilal-alsharifi
Copy link
Contributor

@mrapitis everything looks good to me now. Thanks.

@joeygrover joeygrover merged commit bb58ea2 into develop Jun 27, 2018
@joeygrover joeygrover deleted the feature/issue_552 branch June 27, 2018 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@askirk askirk askirk approved these changes

@bilal-alsharifi bilal-alsharifi bilal-alsharifi approved these changes

@BrettyWhite BrettyWhite Awaiting requested review from BrettyWhite

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mrapitis @codecov-io @bilal-alsharifi @askirk @joeygrover @anildahiya