Extensions list should be an array

[dboulware]

SigMF describes the extensions as a "JSON array of name/value pairs describing SigMF Extension namespaces, where the name is the namespace provided by an extension and the value is a string that specifies whether the extension is optional or the version of the extension required to properly parse & process the SigMF Recording." However, in the example that is shown, it is an object with key value pairs where the key is the namespace provided by an extension and the value is a string that specifies whether the extension is optional or the version of the extension required to properly parse & process the SigMF Recording. When binding the JSON to objects, this requires providing a custom serializer and deserializer. If it were just changed to something like:
"core:extensions" : [
{ "name": "extension-01", "version": "optional" },
{ "name": "extension-02", "version": "v1.2.3" }
]
you would not have to create a custom serializer and deserializer.

[bhilburn]

Hey @dboulware! Thanks so much for opening this.

A few things:

1. Great catch regarding the mismatch between describing it as an array, but defining it as an object! @n-west and I both agree with you that it should be an array. I'll include this in a PR I'll put together, along with some other changes listed below.

2. I don't think I follow the bit about a custom serializer / deserializer. If you paste the following:

{"core:extensions" : {
      "extension-01": "optional",
      "extension-02": "v1.2.3"
    }}

Into this online JSON parser, it's valid JSON. We are going to move this over to an array, anyway, per the first item, but I did want to double-check this to make sure I full understood the issue in case it's still relevant.

There is a third piece to this, too. We discussed this at GRCon18 in the SigMF working group, but the list of extensions should really provide a URI to a machine-readable JSON schema that defines extensions. This would make it possible for SigMF tools to automatically pull and parse full extensions. I'll handle this in a separate issue.

Edit: see #104

[djanderson]

@bhilburn looks like you had a copy/paste issue above.

Were you suggesting something like:

[
    {"extension-01": "optional"},
    {"extension-02": "v1.2.3"}
]

Although really concise notation, I agree with what @dboulware says about that being harder to parse for many JSON serializer/deserializer libraries. Instead of having a variable key, using known keys with known value types tends to be a lot easier.

And while we're discussing this part of the spec, I think it would even be worth it to break out "optional-ness" from the version. What if you want to specify the version of something that's optional?

Recommended:

{
    "extensions": [
        {
            "name": "extension-1",
            "version": "v0.3",
            "optional": true
        },
        {
            "name": "extension-2",
            "version": "v1.2.3",
            "optional": false
        },
    ]
}

[bhilburn]

@djanderson - Whoops, yes. Strangely, it actually wasn't even a typo - all the text was there, but apparently with GH-Flavored MD if there isn't a new line between the ``` and the first text, it hoses the code render, hah.

Anyway, it's fixed now. What I was trying to show was that what's currently in the the specification appears to be valid JSON to me, and that I didn't understand @dboulware's comment about needing a custom serializer for it.

The comment you made in your response regarding known / unknown keys I do follow. And excellent point regarding the fact that something being optional isn't mutually exclusive from needing a specified version. That seems rather obvious now that you've pointed it out.

@dboulware @djanderson - This issue has been really great. So, I think there are two major tasks coming out of this:

• Move the extensions list to an actual JSON array
• Break version out from optional.

[bhilburn]

Please head over to #105, check out the changes, and comment!

[bhilburn]

Tagging this for v0.0.2

[n-west]

This is a pretty solid improvement, and I agree with comments on the serialization/deserialization being a bit more sane with this proposal. I also like that it doesn't break parsing on the legacy field 👍 since it would be an added object rather than changing the type of the existing field

[bhilburn]

This suggestion has been implemented as of #105 merge