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

Misc comments #17

Open
fosrias opened this issue Jul 21, 2016 · 1 comment
Open

Misc comments #17

fosrias opened this issue Jul 21, 2016 · 1 comment

Comments

@fosrias
Copy link

fosrias commented Jul 21, 2016 •
edited
Loading

The following links in blame mode so I can reference explicit lines in your .adoc file.

Is there a reason for not supporting using a version member optionally in the top-level meta object of a root object? I can see a lot of advantages to that in that the content is self-versioning with the default being 1 if absent. Future versions could use it. Some interesting discussions around this in JSON API spec if you search on version.

  • Any reason you have left out a default Form Field Member Type and Form Field Options Types?

Think it would be useful. May want a selected as well. That could be implied by the value being set, so may not be necessary.

Is this default? If so, I would call it that. If not, this is really confusing what it is for.

Find this a bit confusing. Seems like this should be reworded to be SHOULD NOT or possibly it is MUST NOT. I have seen people use MAY NOT even though is not in RFC2119 as the MAY implies that aspect. Another option.

Also, I find the exceptions discussion in the forms section about nested forms really hard to picture. I think you need to create some examples. My mind in that whole section goes "WTF, skip."

I think getting some "MUST ignore" wording in here for emphasis would help.

Location singular? Or reword? Something not english here.

data is redundant. How about "preferred format for data exchange."

I find these exceptions or constraints just being text not really stand out as constraints. I think if I was a client developer, it would be really easy to miss these variations. I think it would be good throughout the document to consistently show these constraints as bullets or numbered lists or something to make them really clear to client library implementers and consumers.

You do it in some places, but not consistently IMO.

Here and elsewhere.

Reference the variant of RegEx? Too lazy to look up, but probably could find in HTML5 spec. Ignore this if they don't do it.

I think the wording should be bolstered to get across the idea that they must not render the value. However, it seems to me that anyone with a browser can introspect the underlying API call and see the payload. So, not sure if this makes sense with respect to not showing the value delivered. If it is meant to hide it when filling in, may want to clarify. Maybe I misunderstand.

You probably should mention the spec does not modify the registered meanings of these relation types and the information is just to clarify their use in ion.

Not going to accommodate non-http protocols and methods?

IMO, returns is more intuitive and simple.
@lhazlewood
Copy link
Member

lhazlewood commented Aug 12, 2016 •
edited
Loading

Hi @fosrias (cc @smizell) great questions! Now that I'm back from SpringOne, I can address them :) Please see inline:

The following links in blame mode so I can reference explicit lines in your .adoc file.

Is there a reason for not supporting using a version member optionally in the top-level meta object of a root object? I can see a lot of advantages to that in that the content is self-versioning with the default being 1 if absent. Future versions could use it. Some interesting discussions around this in JSON API spec if you search on version.

The only real reason is I assume parsers might need to know how to parse the document before they start reading bytes. We could always add it as a supported meta field, maybe ionVersion - I have no strong preference here and would look to your suggestions and the community in general to determine what is preferred.

Any reason you have left out a default Form Field Member Type and Form Field Options Types?

Think it would be useful. May want a selected as well. That could be implied by the value being set, so may not be necessary.

The available field members were purposefully defined to be as similar (where possible) to what HTML 5 supports so transliteration between the two - highly desirable for forms/field data models - could be possible. HTML 5 has no such true default concept: it just has a value attribute, that if populated is the 'default' and the user agent is required to remember that value and re-populate it if the user hits 'clear' on the form. Instead, HTML 5 has a placeholder attribute to give user hints as to what might/should be entered, which ties in to your next question ;)

As far as selected, this isn't necessary because selected options will be represented in the value member. This ensures HTML transliteration is possible, but I posit that Ion's approach is 'cleaner' than HTML's (since Ion bends more on data fields/values than on user-interface controls).

https://github.com/ionwg/ion-doc/blame/master/draft-ion.adoc#L740

  • Is this default? If so, I would call it that. If not, this is really confusing what it is for.

It is to to allow the same behavior in a user agent (if desired or supported by the user agent) as what the HTML 5 placeholder attribute is. It's like an example of what could/should be entered as a value.

https://github.com/ionwg/ion-doc/blame/master/draft-ion.adoc#L235
Need to correct HATEOS to be HATEOAS in the link name.

Ack - please open a PR if I don't get to it first?

https://github.com/ionwg/ion-doc/blame/master/draft-ion.adoc#L452

Find this a bit confusing. Seems like this should be reworded to be SHOULD NOT or possibly it is MUST NOT. I have seen people use MAY NOT even though is not in RFC2119 as the MAY implies that aspect. Another option.

Also, I find the exceptions discussion in the forms section about nested forms really hard to picture. I think you need to create some examples. My mind in that whole section goes "WTF, skip."

I didn't use uppercase terminology because I didn't want any requirement semantics to be present. It is more of a note that "hey, if you see a nested form, it may not be a linked form, so be aware of that".

And you're right about examples and nested forms potentially being confusing. They exist to allow complex object graph definitions to be created and submitted. For example, assume a REST endpoint is expecting a JSON submission that looks like the following:

{
    "user": {
        "givenName": "Joe",
        "surname": "Smith",
        "address": {
            "street1": "1234 Anywhere Street",
            "street2": "Suite 5000",
            "city": "Anytown",
            "state": "Florida",
            "postalCode": 12345,
            "country": "US"
        },
        "employer": {
            "name": "GreatCompany, Inc",
            "address": {
                "street1": "9876 Company Street",
                "city": "Anytown",
                "state": "Florida",
                "postalCode": 12345,
                "country": "US"
            }
        }
    } 
}

In order for a REST client to submit this representation, it needs to know the user format via a form. But in the user format, address and employer themselves have a specific format that needs to be discoverable (as does employer.address). Each nested object within a form needs to be discoverable, and nested forms serves this purpose.

This is much more powerful than HTML forms which only realistically support 'flat' data models - not strong enough IMO for a general purpose data interchange format.

Does this make sense? Any issues you see with this?

I think getting some "MUST ignore" wording in here for emphasis would help.

Ack - please issue a PR if you can.

Location singular? Or reword? Something not english here.

The sentence as written makes sense to me - I'm trying to get across the point that, even if there are multiple nested forms, the entire object graph is submitted to the single top-most form linked location. Does this make sense? If not, any suggestion on how to re-word it?

data is redundant. How about "preferred format for data exchange."

Agreed - good catch. PR please :) (and if you don't, it's ok, I'm just using PR as a marker to help me remember what is needed later).

I find these exceptions or constraints just being text not really stand out as constraints. I think if I was a client developer, it would be really easy to miss these variations. I think it would be good throughout the document to consistently show these constraints as bullets or numbered lists or something to make them really clear to client library implementers and consumers.

You do it in some places, but not consistently IMO.

Definitely fair point, but it's a bit hard for me to see what you mean here. Can you please offer a PR with suggested edits so I can see the kind of changes you have in mind?

may => MAY

Ack. PR.

Reference the variant of RegEx? Too lazy to look up, but probably could find in HTML5 spec. Ignore this if they don't do it.

Ack - I haven't checked myself yet and was wondering what we should mandate/specify.

It turns out HTML5 mandates the JavaScript Pattern definition:

"If specified, the attribute's value must match the JavaScript Pattern production. [ECMA262]"

I'm good with using this if it makes the most sense.

I think the wording should be bolstered to get across the idea that they must not render the value. However, it seems to me that anyone with a browser can introspect the underlying API call and see the payload. So, not sure if this makes sense with respect to not showing the value delivered. If it is meant to hide it when filling in, may want to clarify. Maybe I misunderstand.

It basically again is intended to do what user agents in HTML do today. You can 'view source' and still see the raw value. Is this something that you think we need to change for Ion?

You probably should mention the spec does not modify the registered meanings of these relation types and the information is just to clarify their use in ion.

The Metadata Registry doesn't define link relation types, so I'm a bit confused by this statement :)

Not going to accommodate non-http protocols and methods?

I'm not against it, I was just defaulting to HTTP for convenience. How would you define this section if we were to go beyond HTTP? Could you submit a PR?

IMO, returns is more intuitive and simple.

I initially chose produces as I thought it was the most logical word pair/parallel to accepts, which is what most people think of (e.g. the Accept header). I'm open to whatever as long as the the two are logical pairs. return doesn't feel quite right here because this is is supposed to represent the media type of the content, not the content itself. Might there be a better word fit?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@fosrias @lhazlewood