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

feat: improve headers guideline (#769) #769

Merged
merged 1 commit into from
Jul 21, 2023
Merged

feat: improve headers guideline (#769) #769

merged 1 commit into from
Jul 21, 2023

Conversation

tkrop
Copy link
Member

@tkrop tkrop commented Jul 12, 2023

This pull request picks up a couple of header related tasks and left overs from previous pull request, e.g. #760, and from the rolling agenda to complete default header definition support via the guidelines:

  • Add general introduction on how to reference default header definitions from the guideline.
  • Add script to generate guideline definition of headers from provided default models to use models as a single source of truth.
  • Add support for Idempotency-Key, Prefer, Accept-Encoding, Content-Encoding, Sunset, and Deprecation headers.
  • Improve support and correct definitions of If-Match, If-None-Match, ETag , Cache-Control, and Vary headers.
  • Replace references to RFCs obsoleted by RFC-9110 and RFC-9111 - except of status code and error codes section.

Generally, this pull request is editorial and does not require Zally to be changed, however, it would be good to add a hint about compression as a Zally rule, since we now can identify endpoints not supporting compression.

Comment on lines +20 to +27

[[using--headers]]
== Using Standard Header definitions

Usually, you can the standard HTTP request and response header definition
provided by the guideline to simplify API by using well recognized patterns.
The best practice importing headers providing the highest readability is as
follows:
Copy link
Member Author

Choose a reason for hiding this comment

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

May be we should introduce this as a new rule to reference headers - if possible - this way.

Comment on lines +222 to 227
[source,yaml]
----
include::../includes/etag.yaml[]
include::../includes/if-match.yaml[lines=3..-1]
include::../includes/if-none-match.yaml[lines=3..-1]
----
Copy link
Member Author

Choose a reason for hiding this comment

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

In the long run, I want this represented as collapsible block, that can be unfolded on demand. Since this currently does not work with our current asciidoc stile, I think, we need to keep this visible by default.

@tfrauenstein
Copy link
Member

👍

Copy link
Member

@tfrauenstein tfrauenstein left a comment

Choose a reason for hiding this comment

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

Thank you! Looks good!

tkrop pushed a commit that referenced this pull request Jul 20, 2023
Co-authored-by: Paŭlo Ebermann <paul.ebermann@zalando.de>
@tkrop tkrop force-pushed the improve-headers branch from 9690f26 to 269f53b Compare July 20, 2023 12:25
@tfrauenstein
Copy link
Member

👍

1 similar comment
@tkrop
Copy link
Member Author

tkrop commented Jul 21, 2023

👍

@tkrop tkrop merged commit 873f34f into main Jul 21, 2023
@tkrop tkrop deleted the improve-headers branch July 21, 2023 19:27
tkrop pushed a commit that referenced this pull request Sep 15, 2023
tkrop pushed a commit that referenced this pull request Sep 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants