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

Encoding Object content and header clarifications (3.0.4) #3857

Merged
merged 7 commits into from
Jun 12, 2024

Conversation

handrews
Copy link
Member

@handrews handrews commented May 25, 2024

Rendering with all currently open 3.0.4 changes (as of the timestamp of this edit - use the dropdown on the word "edited" in the bar for this comment) can be found here.

Fixes:

The Encoding Object's contentType field takes a comma-separated list of either regular or wildcared media types. These are the "two types" mentioned in the previous wording – "two" here did not refer to a limit on the number of entries in the list.

These are not exactly media-type or media-range values, as both of those include parameters.

This change also moves the hard-to-follow list of default values out of the individual field cell and into its own table.

It takes Content-Disposition out of the header field's cell and instead explains limitations on header usage, and explains how Content-Disposition is used for encoding.

This explanation includes a suggestion on how other multipart formats could be used with an Encoding Object, since their unnamed parts otherwise cannot be supported.

Finally, it clarifies the interaction between contentType and the three fields imported from the Parameter Object, by aligning the recommended (but not, for compatibility reasons, required) behavior with guidance added in 3.1.0.

@handrews handrews added the media and encoding Issues regarding media type support and how to encode data (outside of query/path params) label May 25, 2024
@handrews handrews added this to the v3.0.4 milestone May 25, 2024
@handrews handrews requested a review from a team May 25, 2024 22:43
@handrews handrews added param serialization Issues related to parameter and/or header serialization clarification requests to clarify, but not change, part of the spec and removed param serialization Issues related to parameter and/or header serialization labels May 25, 2024
versions/3.0.4.md Outdated Show resolved Hide resolved
versions/3.0.4.md Outdated Show resolved Hide resolved
versions/3.0.4.md Outdated Show resolved Hide resolved
ralfhandl
ralfhandl previously approved these changes Jun 4, 2024
versions/3.0.4.md Outdated Show resolved Hide resolved
versions/3.0.4.md Outdated Show resolved Hide resolved
Copy link
Contributor

@notEthan notEthan left a comment

Choose a reason for hiding this comment

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

looks good

ralfhandl
ralfhandl previously approved these changes Jun 7, 2024
@handrews
Copy link
Member Author

@notEthan @ralfhandl I'm afraid I did a little re-arranging after resolving conflicts because it got less clear. This now splits the fields like they are in the Parameter Object and the Header Object (now that both of those have their own tables structured the same way, the Encoding Object became a weird exception to the pattern).

This will conflict with the percent-encoding PR, but it would have anyway. I'll resolve conflicts for whichever isn't merged first.

Copy link
Contributor

@ralfhandl ralfhandl left a comment

Choose a reason for hiding this comment

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

Minor nits

versions/3.0.4.md Outdated Show resolved Hide resolved
versions/3.0.4.md Outdated Show resolved Hide resolved
ralfhandl
ralfhandl previously approved these changes Jun 11, 2024
@ralfhandl ralfhandl requested a review from a team June 11, 2024 17:11
handrews and others added 4 commits June 11, 2024 11:04
The Encoding Object's `contentType` field takes a comma-separated
list of either regular or wildcared media types.  These are the
"two types" mentioned in the previous wording – "two" here did
*not* refer to a limit on the number of entries in the list.

These are not exactly media-type or media-range values, as both
of those include parameters.

This change also moves the hard-to-follow list of default values
out of the individual field cell and into its own table.

It takes `Content-Disposition` out of the header field's cell
and instead explains limitations on header usage, and explains
how `Content-Disposition` is used for encoding.

This explanation includes a suggestion on how other `multipart`
formats could be used with an Encoding Object, since their
unnamed parts otherwise cannot be supported.

Finally, it clarifies the interaction between `contentType`
and the three fields imported from the Parameter Object, by
aligning the recommended (but not, for compatibility reasons,
required) behavior with guidance added in 3.1.0.
Co-authored-by: Ralf Handl <ralf.handl@sap.com>
handrews and others added 3 commits June 11, 2024 11:04
This splits the Encoding Object's fixed fields table
to make the usage more clear, and closer to how it is presented
for the Parameter and Header Objects
🤦 thanks for catching this, @ralfhandl

Co-authored-by: Ralf Handl <ralf.handl@sap.com>
@ralfhandl ralfhandl requested a review from a team June 12, 2024 07:20
@miqui miqui merged commit 3734678 into OAI:v3.0.4-dev Jun 12, 2024
1 check passed
@handrews handrews deleted the enc-304 branch June 13, 2024 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification requests to clarify, but not change, part of the spec media and encoding Issues regarding media type support and how to encode data (outside of query/path params)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants