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

EZP-29977 [Spike] Use RAML for REST API reference - DO NOT MERGE #518

Closed
wants to merge 7 commits into from

Conversation

MagdalenaZuba
Copy link
Contributor

This PR covers tests for documenting API reference with RAML - DO NOT MERGE

Small section of REST API reference: Bookmark, Content and User was rewritten into RAML.
HTML output was generated from it using raml2html.

links:
how to install RAML: https://www.npmjs.com/package/raml2html
where to get raml2html-material-theme from: https://www.npmjs.com/search?q=keywords:raml2html-theme

command:
raml2html --theme raml2html-material-theme ez.raml > ez.html

RAML/ez.raml Outdated
description: List the root resources of the eZ Publish installation.
headers:
Accept:
description: application/vnd.ez.api.BookmarkList+xml or application/vnd.ez.api.BookmarkList+json
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
description: application/vnd.ez.api.BookmarkList+xml or application/vnd.ez.api.BookmarkList+json
description: application/vnd.ez.api.Root+xml or application/vnd.ez.api.Root+json

RAML/ez.raml Outdated
description: Error - if a given location not exists / is not bookmarked
/Content:
get:
description: List the root resources of the eZ Publish installation.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
description: List the root resources of the eZ Publish installation.
description: List the root resources of the eZ Platform installation.

Copy link
Member

@alongosz alongosz left a comment

Choose a reason for hiding this comment

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

This looks very good both for writing and reading that documentation! 💖
Much more manageable than reStructuredText IMHO.

RAML/ez.raml Outdated
description: Error - if the user is not authorized to given location
404:
description: Error - if a given location not exists / is not bookmarked
/Content:
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick:

Suggested change
/Content:
/content:

we always use lowercase in path elements.

RAML/ez.raml Outdated
description: Temporary redirect
404:
description: Error - if the content with the given remote ID does not exist
/objects/{id}:
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: this might be a good chance to improve readability (either now or as a follow-up). Maybe {contentId} instead, so we exactly know what that is.

RAML/ez.raml Outdated
<names>
<value languageCode="ger-DE">Neuer Titel</value>
</names>
<Content href="/content/objects
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: missing end of line, it might be important to some text parsers, that's why GitHub marks it in a specific way.

Copy link
Member

@adamwojs adamwojs left a comment

Choose a reason for hiding this comment

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

This is REST API documentation which I expect as a developer!

The only missing thing is data types spec for request payload and response. Ref. https://github.com/raml-org/raml-spec/blob/master/versions/raml-10/raml-10.md/#raml-data-types

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Wait with merge PRs that shouldn't be merged instantly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants