-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Conversation
When I think about it more conceptually – the endpoint as such is actually an action section in API Blueprint parlance (albeit it implies a new resource). Therefore it should be defined under Action and not resource as such. This, however, has some implications – suddenly we can have "action" section at the top of a blueprint so the "Blueprint document structure" needs to be updated accordingly. Internally, in the parser, this should be still picked up by Resource parser as it essentially creates a new resource (in AST) but this is opaque to user. Furthermore parser should not warn when an action with the same URI template (but different method) is described, instead, it should add it under the respective resource. Overall as simply as it looks it is actually quite complex problem :) |
Hmm, I guess that in that case we'd want to change the behaviour of I'm not sure if I'm happy with letting content "jump" to different places thought, it would make the internals quite a bit more complicated? It would also make it complicated to do "guides" with blueprint since the content might not necessarily come in the order that you specify it in the markdown. e.g. # Create a user [POST /users]
...
# Log in as the user [POST /sessions]
...
# List all users [GET /users]
... This would now list |
|
||
+ Request | ||
+ Headers | ||
Content-Type: application/json |
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.
The content inside Headers
and Body
list items should be code blocks. So, indent them by 12 spaces.
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.
👍
A few styling issues other than looks good. But I agree with @zdne that even though this issue looks simple, it is a bit complex one. We need to do some thinking on this. |
|
||
Defined by a resource [name (identifier)](#def-identifier) followed by an [HTTP request method][httpmethods] and an [URI template][uritemplate] enclosed in square brackets `[]`: | ||
|
||
# <identifier> [<HTTP request method> <URI template>] |
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.
By the way, if we are doing this, please add some info about this to the section title Header Keywords
.
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.
@pksunkara I think it is OK to leave it as it is, because it is being already covered by "Combinations of an HTTP method and URI Template (e.g. GET /resource/{id})"
Hey @LinusU I was thinking about it a bit more and I think you are indeed right with:
Now doing that and also addressing
is quite radical change... So lets recap on what are the options: 1. Add name to an endpointSimply allow adding a name to Pros
Cons
2. Separate endpoints from resourcesIntroduce a new top-level "endpoint" section – for Pros
Cons
Thoughts ? |
@zdne great writeup. Number 1 is what I have implemented in this pull request, the only change really is to allow a name to the resource that gets generated. I do like number 2 quite much thought, but as you say it's much more work involved. I think that we need to clarify a bit an endpoint is. Is it a path (e.g. If we try and work our way from the ast, which I think is a good approach, how would the new ast structure look? I guess that it looks something like this currently:
And it would become something like this if we added endpoints:
Or maybe
Very simple ast at least. Every endpoint should then contain Maybe a good approach would be to implement number 1 now since that dosen't change anything for the existing ecosystem, and then try and figure out what we think about number 2. I really like the ideas in number 2 thought, it makes it possible to specify APIs that aren't RESTful which I think is important. |
@LinusU OK so let's break it into two parts
|
👍 In the current context I am using the term "endpoint" for a method + URI pair
👍 Note @smizell is currently working Refract Resource Namespace – this eventually should be the underlaying data structure – I believe he will address these main concept there (resources and transition) using some sort of recursive pattern. TL;DR let's figure this one (point 2 as mentioned above) in the Resource namespace first and focus on just adding names to currently un-named method URI pairs. Sounds good? |
+ Headers | ||
Content-Type: application/json | ||
+ Body | ||
{ "username": "Linus", "password": "test" } |
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.
Consistence: Can we please use the same / similar example data as in the previous examples?
@LinusU Please address the review comments on the example and it is good to go! Thanks! 🎈 |
@zdne Should be good to go 👍 |
|
||
# <identifier> [<HTTP request method> <URI template>] | ||
|
||
> **NOTE:** In the two latter cases the rest of this section represents the [Action section](#def-action-section) including its description and nested sections and **follows the rules of Action section instead**. |
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.
- In the two latter
+ In the latter two
@pksunkara Done! |
Looks good. I am toggling this as "Passed Review". 👍 |
@zdne Since I merged apiaryio/snowcrash#332, Do we merge this? Or should we wait for releasing it on Apiary? |
@pksunkara I think we can merge this / update README (version supported in apiary) and release new version of API Blueprint... |
@LinusU Can you please make a new commit to this PR updating the current format to 1A9 from 1A8? Thanks. |
I'm on it! |
Done, I did not tag it since it seems that you have always tagged the merge commit :) |
Cheers 🍻 |
See discussion in #196