-
Notifications
You must be signed in to change notification settings - Fork 8
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
Renamed name
to title
and added lang
& dir
.
#225
Conversation
"locale" is probably not the right name for this field, as it implies something about the processing environment or maybe the configuration of the host serving the credentials. A better model would probably be to follow some other specs and use Providing for multilingual |
name
to title
and added locale
.name
to title
and added lang
& dir
.
Thanks @aphillips , I made the changes so we use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great. Thanks for the changes so far.
<a data-cite="ltli/#dfn-language-tag">language tag</a> for the values of the [=title=] and | ||
[=description=] properties defined in the JSON Schema. The value of this field MUST be a | ||
<a data-cite="ltli/#dfn-canonical-unicode-locale-identifier">canonical unicode locale identifier</a>. | ||
When absent, implementers MUST assume that the value of this property is `en-US`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't agree with this in app-manifest (I think we have an issue open against it?). The value when the language is not present should be und
("Undetermined", also known as the "root" locale)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what issue/repo you may be referring to. Mind clarifying?
Curious, is that und
guidance part of the LTLI spec?
I've updated this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious, is that und guidance part of the LTLI spec?
Actually, no, it's in String-Meta and the I18N Best Practices for Specs document. I've created an issue to add it to LTLI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approving pending @aphillips 's recommended changes. thanks for getting this up!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good. Thanks!!
Fixes #224 by adding a
locale
field.Additionally, this PR also renames
name
in favor oftitle
, because the latter is part of the json schema spec.Preview | Diff