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

[Accepted with Revisions] SDL 0072 - New vehicle data - FuelRange #207

Closed
theresalech opened this issue Jun 28, 2017 · 7 comments
Closed

Comments

@theresalech
Copy link
Contributor

Hello SDL community,

The review of "New vehicle data - FuelRange" begins now and runs through July 5, 2017. The proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0072-New-vehicle-data-FuelRange.md

Reviews are an important part of the SDL evolution process. All reviews should be sent to the associated Github issue at:

#207

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of SDL. When writing your review, here are some questions you might want to answer in your review:

  • Is the problem being addressed significant enough to warrant a change to SDL?
  • Does this proposal fit well with the feel and direction of SDL?
  • If you have used competitors with a similar feature, how do you feel that this proposal compares to those?
  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
    Please state explicitly whether you believe that the proposal should be accepted into SDL.

More information about the SDL evolution process is available at

https://github.com/smartdevicelink/sdl_evolution/blob/master/process.md

Thank you,
Theresa Lech

Program Manager - Livio
[email protected]

@joeygrover
Copy link
Member

<description>
       Is related to vehicles equipped with a tank for compressed natural gas.
</description>

This descriptions should be only for the type of fuel, we don't need to say Is related to vehicles equipped with a tank for.

<struct name="FuelRange">
    <param name="type" type="FuelType" mandatory="true"/>
    <param name="range" type="Float" minvalue="0" maxvalue="10000" mandatory="true">
        <description>
            The estimate range in KM the vehicle can travel based on fuel level and consumption.
        </description>
    </param>
</struct>

We should aim to only make this mandatory when they are absolutely mandatory. This helps us not make breaking changes in the future if we don't need to. I would recommend that FuelType is not mandatory. I'd like see range not mandatory as well so in the future if we need to make changes we can add a different param rather than breaking this one, but it is not hard requirement from me.

This proposal needs a few minor adjustments, but overall should be accepted.

@tpulatha
Copy link
Contributor

I agree to the not mandatory nature of these arguments. It should probably be a rule to not have vehicle data items mandatory. Apps have to check if the values they need are available and put in fallbacks if needed, as this can happen for multiple reasons (headunit does not support value, user did not consent, etc)

@joeljfischer
Copy link
Contributor

I agree with Joey about making it not mandatory, and that it is a good idea for implementation.

@theresalech theresalech changed the title [In Review] SDL 0072 - New vehicle data - FuelRange [Accepted with Revisions] SDL 0072 - New vehicle data - FuelRange Jul 6, 2017
@theresalech
Copy link
Contributor Author

The Steering Committee has voted to accept this proposal with revisions. It was agreed that FuelType and range be optional and not mandatory, based on the reasons provided in the comments on the issue.

@theresalech
Copy link
Contributor Author

@robinmk - please advise when a new PR has been entered to update the proposal to reflect the agreed upon revisions (arguments being optional instead of mandatory). I'll then merge the PR so the proposal is up to date, and enter issues in respective repositories for implementation. Thanks!

@theresalech
Copy link
Contributor Author

Proposal has been updated to reflect revisions agreed upon by Steering Committee.

@theresalech
Copy link
Contributor Author

theresalech commented Jul 10, 2017

Issues Entered:
Core
iOS
Android
RPC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants