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

app_id value is case sensitive as suggested in #7 #58

Merged
merged 3 commits into from Nov 25, 2022
Merged

app_id value is case sensitive as suggested in #7 #58

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b467195
Case sensitive as suggested in #7
espinr Sep 8, 2022
039e1a0
Add description in the JSON schema
espinr Sep 8, 2022
bf459c4
app_id "is" following the infra definition
espinr Sep 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion index.html
View file
Open in desktop
Expand Up @@ -787,7 +787,7 @@ <h3>
The [=MiniApp manifest's=] <code><dfn>app_id</dfn></code> member is a [=string=] that identifies the MiniApp univocally. This member is mainly used for package management, and it supports the update and release process of MiniApp versioning.
Copy link

Choose a reason for hiding this comment

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

The word "univocally" is used correctly here, but, as a native English speaker, it's an extremely rare word (I had to look it up). I would suggest replacing it with the much more common "uniquely" or, perhaps, univocally's synonym "unequivocally".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you @aphillips . Changed as suggested.
I don't know where I learned that word. I've been using it for a while... thinking it was totally valid :-) Changed also in my dictionary :-)

</p>
<p>
The format of <code>[=app_id=]</code> is RECOMMENDED to be a [=string=] defined by the following rule:
The value of <code>[=app_id=]</code> is RECOMMENDED <a data-link-type="dfn" href="https://infra.spec.whatwg.org/#string-is">to be</a> a [=string=] defined by the following rule:
Copy link

Choose a reason for hiding this comment

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

RECOMMENDED is the equivalent of SHOULD, not MUST, which means that this is not really a requirement--it's more of a suggestion. If you mean to define a strict namespace (and I think you do to ensure interoperability), you should consider using MUST or revising this requirement to just say:

The value of app_id is a string whose contents are defined by the following rule:

Note that the link to Infra's "string is" rule makes no sense here. The Infra definition is about string equivalence, not content restriction.

Copy link
Collaborator Author

@espinr espinr Dec 12, 2022 •
edited

Choose a reason for hiding this comment

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

I've tried to add it somehow but the 'is' equals to 'is equivalent to' so it does not fit. I've changed it for a case-sensitive string.

Using SHOULD instead of RECOMMENDED. I understand this could be any string, but we want to recommend the way some mini apps usually do.

Copy link

Choose a reason for hiding this comment

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

I probably didn't explain clearly what I meant.

The Infra reference string-is has to do with comparing two strings for equality, not defining what a given string field may consist of. One would use string-is to write a requirement like:

The value of value of the key is the string foo

Here you are trying to define the namespace of app_id, so you shouldn't use the string-is definition.

If app_id can be any string and there are no actual requirements on content, I think you need to very careful to spell that out. How will you write the conformance tests? How will implementations deal with apps that do not follow the recommendation? For the miniapp space to work reliably, the spec needs to be super clear.

</p>
<pre class="abnf">
appIdRule = name *("." name)
Expand Down
2 changes: 1 addition & 1 deletion manifest_schema.json
View file
Open in desktop
Expand Up @@ -85,7 +85,7 @@
},
"app_id": {
"type": "string",
"description": "A string that identifies the MiniApp univocally.",
"description": "A case sensitive string that identifies the MiniApp univocally.",
Copy link

Choose a reason for hiding this comment

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

See above about "univocally"

I would probably change this to read more like:

A string containing a unique identifier for the MiniApp; case-sensitive.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed as suggested. Thanks.

"example": "org.example.miniapp"
},
"color_scheme": {
Expand Down