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] Revise SDL-0238 Keyboard Enhancements #1119

Closed
jordynmackool opened this issue Jan 27, 2021 · 7 comments
Closed

[Accepted with Revisions] Revise SDL-0238 Keyboard Enhancements #1119

jordynmackool opened this issue Jan 27, 2021 · 7 comments

Comments

@jordynmackool
Copy link
Contributor

jordynmackool commented Jan 27, 2021

Hello SDL community,

The review of "Revise SDL-0238 Keyboard Enhancements" begins now and continues through February 02, 2021.

This will be a review of proposed revisions to a previously accepted but not yet implemented proposal, SDL 0238.

The pull request outlining the revisions under review is available here:

#1117

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

#1119

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,
Jordyn Mackool

Program Manager - Livio
jordyn@livio.io

@joeljfischer
Copy link
Contributor

1. It's a little hard to keep track of differences with everything next to each other as it is. I think KeyboardProperties.customKeys has two descriptions right now.

2. I think that to prevent confusion between KeyboardCapabilities and KeyboardCapability we could perhaps change KeyboardCapability to KeyboardConfiguration?

@jacobkeeler
Copy link
Contributor

@joeljfischer

  1. customKeys does have two descriptions, but this was also the case before the revision. There are other parameters in the spec with multiple <description> tags (see limitedCharacterList) and these are just treated as multiline descriptions. That said, I could just change this to use a single multiline <description> tag, if that's preferable.
  2. I think I was following the pattern used by app services for this situation (AppServiceCapabilities vs. AppServiceCapability), though it doesn't look like this naming scheme is used elsewhere in the spec. I still think this struct should include "capability" in its name, so perhaps KeyboardLayoutCapability (or KeyboardLayoutCapabilities) would be preferable?

@joeljfischer
Copy link
Contributor

1. I think that would probably be preferable. I hadn't noticed anything with two descriptions before, but I guess it does happen. It's a bit odd though.

2. I'm okay with that.

@jacobkeeler
Copy link
Contributor

Alright, so as of now the additional revisions planned are:

  1. Change customKeys to use a single <description> tag
  2. Rename KeyboardCapability to KeyboardLayoutCapability

@jordynmackool
Copy link
Contributor Author

The Steering Committee voted on 2021-02-02 to accept the revised proposal with the revisions summarized in this comment.

@jordynmackool
Copy link
Contributor Author

jordynmackool commented Feb 3, 2021

@jacobkeeler please advise when the PR has been updated to reflect the agreed-upon revisions. I'll then merge the PR so the proposal is up to date, and comment on the already created issues for the impacted repositories. Thanks!

@smartdevicelink smartdevicelink locked and limited conversation to collaborators Feb 3, 2021
@jordynmackool jordynmackool changed the title [In Review] Revise SDL-0238 Keyboard Enhancements [Accepted with Revisions] Revise SDL-0238 Keyboard Enhancements Feb 3, 2021
@jordynmackool
Copy link
Contributor Author

The proposal has been updated based on the Steering Committee's agreed-upon revisions. Comments have been left on implementation issues to reference revisions:

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

No branches or pull requests

3 participants