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

SDL-0188 Interior Vehicle Data resumption revision #1062

Conversation

LuxoftAKutsan
Copy link
Contributor

@LuxoftAKutsan LuxoftAKutsan commented Aug 4, 2020

Introduction

Update "SDL-0188 Interior Vehicle Data resumption." The revision is in the scope of fixing typos to match the RPC Spec.

Motivation

Fix typos to avoid confusion in further activities.

Proposed solution

Fix parameter names for GetInteriorVehicleData from IsSubscribe to subscribe according to the existing API.

In behaviour changes block, replace SetInteriorVehicleData with the GetInteriorVehicleData RPC that was initially meant.

Potential downsides

N/A

Impact on existing code

N/A

Alternatives considered

N/A

@LuxoftAKutsan
Copy link
Contributor Author

@theresalech please review this proposal revision.
This proposal is intended to be included to 7.0
The revision do not introduce any logical changes to the proposal.
The only fix is types in parameter names and function names.
@yang1070 approved the revision

Copy link
Contributor

@yang1070 yang1070 left a comment

Choose a reason for hiding this comment

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

Ford approves this proposal

Copy link
Contributor

@theresalech theresalech left a comment

Choose a reason for hiding this comment

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

@LuxoftAKutsan, since this is a request to revise a previously accepted proposal, please update the PR description to follow the format of the proposal template. See more information on revising previously accepted proposals here, and an example of how to format this type of pull request here. Thank you!

@LuxoftAKutsan
Copy link
Contributor Author

@theresalech fixed description. Added paragraphs that were changed

@theresalech
Copy link
Contributor

@LuxoftAKutsan, thanks for your updates. However, the PR description still needs to be changed to match the format of a proposal template, and include all sections (Motivation, Proposed solution, Potential downsides, etc.). Please see this example: #775.

@LuxoftAKutsan
Copy link
Contributor Author

@theresalech sorry for confusion. Fixed.

Copy link
Contributor

@theresalech theresalech left a comment

Choose a reason for hiding this comment

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

@LuxoftAKutsan, thanks very much! Can you please make the following updates within the PR description? Let me know if you have any questions or concerns. Thanks again!

(1) In the Introduction section:

Updating SDL-0188 Interior Vehicle Data resumption revision in scope of fixing typos to match rpc_spec.

should be

Update "SDL-0188 Interior Vehicle Data resumption." The revision is in the scope of fixing typos to match the RPC Spec.

(2) In the Proposed solution section:

Fixing parameter names for GetInteriorVehicleData from IsSubscribe to subscribe according existing API.

In behavior changes block replaced SetInteriorVehicleData with GetInteriorVehicleData RPC that was initially meant.

should be

Fix parameter names for GetInteriorVehicleData from IsSubscribe to subscribe according to the existing API.

In behavior changes block, replace SetInteriorVehicleData with the GetInteriorVehicleData RPC that was initially meant.

@LuxoftAKutsan
Copy link
Contributor Author

@theresalech fixed

@LuxoftAKutsan
Copy link
Contributor Author

LuxoftAKutsan commented Aug 20, 2020

@theresalech
I added one mode change to the proposal revision.
There was mentioned that GetInteriorVehicleData should support WARNING result code.
But API changes was not explicitly mentioned.
Commit 6dca09c add this explicit mention
I don't know the correct process for this change considering that this revision is waiting for SDLC.

Update : newer mind reverted this fix to avoid confusion on SDLC

@LuxoftAKutsan LuxoftAKutsan force-pushed the revision/get_interior_data_resumption branch from 6dca09c to bd72b67 Compare August 20, 2020 14:51
@theresalech
Copy link
Contributor

@LuxoftAKutsan, if you would like to see additional changes in the proposed revision, please leave those as a comment on the proposal review issue, here: #1072. This will allow all SDLC members to review and consider in their vote on this revision during our next Steering Committee meeting. Thanks!

@theresalech theresalech merged commit de927ac into smartdevicelink:master Aug 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants