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

Prefixing local records with ":" would be simpler than isLocal #501

Closed
kenchris opened this issue Jan 2, 2020 · 6 comments
Closed

Prefixing local records with ":" would be simpler than isLocal #501

kenchris opened this issue Jan 2, 2020 · 6 comments

Comments

@kenchris
Copy link
Contributor

kenchris commented Jan 2, 2020

I don't like isLocal because its optional so easy to get wrong, which is made clear with the last commits "sprinking it" over existing code :)

":" is not a valid first character for local record types, so it is fine.

I think it makes sense because we have local records in namespaced (external records) or standardized records (smart poster). It looks like a namespace where you just didn't explicitly write it out.

It will make make it very explicit where a local record type is used.

const writer = new NDEFWriter();
writer.push({ records: [
  {
    recordType: "imdb.com:movies",
    data: { records: [
      {
        recordType: ":rating",
        data: 10
      },
      {
        recordType: ":title", 
        data: "Rise of Skywalker"
      }]
   }
}]);
@kenchris
Copy link
Contributor Author

kenchris commented Jan 2, 2020

a recordType: "text" would thus be a normal T record, and recordType: ":text" would be a local record of type "text"

@beaufortfrancois
Copy link
Collaborator

FYI @kenchris #375 (comment) explains why the prefix idea was not picked.

My take: I am not convinced the arbitrary complexity introduced by Web NFC in 2b) is worth doing, for the sake of saving complexity of defining both (standardized, i.e. already known) TNF and TYPE by applications.

@kenchris
Copy link
Contributor Author

kenchris commented Jan 2, 2020

A local type written as "_text" or "local:text" would be specified as "__text" or "local:local:text", respectively.

_ and : are not allowed as first letter in local types, has to be number or lowercase letter, so you would never run into the above issues

@beaufortfrancois
Copy link
Collaborator

For the exercise, I've updated spec to see how it would look: #502

@zolkis
Copy link
Contributor

zolkis commented Jan 2, 2020

Right, I have overlooked that : is illegal starting character for local types.

However, developers might be confused with this new Web NFC specific convention. It is also confusing because : is a separator in external types.

On the other hand, in a way it's a logical choice (local is like external, just that instead of the explicit domain-context we have the implicit local context).

I'd prefer using the NFC Forum standardized type names, but we could assume this as a working hypothesis.

We should make Notes about the Web NFC introduced conventions about type names and ask for developer feedback between this (the current way) or using the plain NFC Forum standardized type names (T, U, Sp, act, etc).

@beaufortfrancois
Copy link
Collaborator

#502 has been merged.
#504 has been opened to track which differences we want to highlight about Web NFC vs NFC Forum spec.
@kenchris Please close this issue at your discretion.

@kenchris kenchris closed this as completed Jan 2, 2020
@zolkis zolkis reopened this Jan 3, 2020
@zolkis zolkis closed this as completed Jan 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants