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

Clarify the behavior of YAML comments in Overlays #83

Open
earth2marsh opened this issue Oct 16, 2024 · 4 comments
Open

Clarify the behavior of YAML comments in Overlays #83

earth2marsh opened this issue Oct 16, 2024 · 4 comments
Milestone

Comments

@earth2marsh
Copy link
Member

The Overlay Specification should clarify how YAML comments work. Some things to consider:

  1. That YAML comments may be used within an overlay document
  2. That any comments in any original YAML description document (i.e., the document that an overlay modifies) should be preserved in the merge
  3. ... or alternatively that you may expect that any comments are nuked after applying an overlay.
  4. Other?
@earth2marsh
Copy link
Member Author

Thomas's comment in a slack thread caused me to check the OpenAPI Specification to see what we say about YAML comments there, and it turns out we don't say anything about them. That makes some sense to me, as I wouldn't expect converting YAML > JSON > YAML would preserve a comment.

However, Overlays feels a little bit different in that you could apply YAML to YAML and end up losing your YAML comments. Isn't that a small surprise?

@handrews
Copy link
Member

My feelings are that:

  1. Anything that can't be used across both JSON and YAML should be outside of our specification(s)
  2. YAML comments in an overlay are comments about the overlay, not the OAS content (because the OAS does not make any mention of YAML comments at all)

@handrews
Copy link
Member

It's worth noting that many YAML packages do not preserve comments, or require great contortions to access them. Here is a thread on comments in PyYAML, the de-facto Python standard for working with YAML (and fastest by a large margin despite the claims of other libraries- I tested a bunch of them).

Therefore, I don't think it's reasonable for any of our specifications to mandate YAML comment-preservation behavior. It's simply not going to be interoperable.

I think this strengthens the argument that, even when using a comment-preserving parser, comments from the overlay should be treated as commenting on the overlay and not as commenting on the target. That makes the behavior between comment-preserving and non-comment-preserving parsers more similar.

I see this as analogous to preservation of line numbers: That's not something any OpenAPI specification (little-s, generic, not just OAS) should mandate. Not only do many parsers not track them, but the ones that do are generally orders of magnitude slower.

@handrews
Copy link
Member

Also, while that thread about PyYAML mentions an alternative package, keep in mind that in many environments, incorporating a new package into code is an extremely heavy-weight process requiring both legal and security review. Anything we mandate needs to work with the most common libraries and not require something unusual.

@ralfhandl ralfhandl added this to the Release 1.0 milestone Nov 8, 2024
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