-
Notifications
You must be signed in to change notification settings - Fork 14
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(guidelines): add guidelines for test header usage in rest apis #65
feat(guidelines): add guidelines for test header usage in rest apis #65
Conversation
api-guidelines/rest/http/headers/rules/must-forward-test-header.md
Outdated
Show resolved
Hide resolved
api-guidelines/rest/http/headers/rules/must-forward-test-header.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Christina Framke <christina.gebken@otto.de>
…r.md Co-authored-by: Christina Framke <christina.gebken@otto.de>
…r.md Co-authored-by: Christina Framke <christina.gebken@otto.de>
Co-authored-by: Christina Framke <christina.gebken@otto.de>
Co-authored-by: Christina Framke <christina.gebken@otto.de>
Co-authored-by: Christina Framke <christina.gebken@otto.de>
Co-authored-by: Christina Framke <christina.gebken@otto.de>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last suggestion.. Sorry :D
api-guidelines/rest/http/headers/rules/must-forward-test-header.md
Outdated
Show resolved
Hide resolved
…r.md Co-authored-by: Christina Framke <christina.gebken@otto.de>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I'd like to support you with this PR, let me "request" these last change as I ran the script to find new available rule IDs.
According changelog suggestion:
Changelog:
### New
- MAY use Test header [R000080](../api-guidelines/rest/http/headers/rules/may-use-test-header.md)
- MUST forward Test header [R000081](../api-guidelines/rest/http/headers/rules/must-forward-test-header.md)
api-guidelines/rest/http/headers/rules/must-forward-test-header.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Christina Framke <christina.gebken@otto.de>
…r.md Co-authored-by: Christina Framke <christina.gebken@otto.de>
Done |
Can you take a look at this PR, @thomasklinger1234 @jensfischer1515, thx :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read the new guideline and find it helpful.
I see additional need for communication between all involved teams if they need to differentiate processing "test" requests and regular requests. I guess there's not automatic way to do that. Should we stress this fact even more?
Hey @ccudennec-otto, do you mean communication after introducing this rule, e.g. in our TD circle? Or do mean we should add more clarification in the description of the rule itself? |
Hi @maxedenharter0507 !
I just wondered what happens if a team uses "Test" headers but consuming team don't consider them. As the header is not restricted to nonlive environments, it could trigger some real-world actions, couldn't it? Maybe those actions even cost real money. That's the potential risk that I see and that's why I thought we could stress this a little more. I'm available for a chat or call - maybe it makes it easier to clarify this issue for me. 🙂 |
api-guidelines/rest/http/headers/rules/must-forward-test-header.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Birgit Bader <birgit.bader@otto.de>
Co-authored-by: Birgit Bader <birgit.bader@otto.de>
Co-authored-by: Birgit Bader <birgit.bader@otto.de>
Co-authored-by: Birgit Bader <birgit.bader@otto.de>
Co-authored-by: Birgit Bader <birgit.bader@otto.de>
We identified that there is no rule about flagging test requests or responses of REST APIs. As we have a guideline for async events the new rules following the same approach.
Open questions in my mind to be discussed:
Changelog:
New