-
Notifications
You must be signed in to change notification settings - Fork 191
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
Docs: extend REST API #4492
Docs: extend REST API #4492
Conversation
@giovannipizzi I have edited the original documentation text only very minimally. |
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 agree that it doesn't make sense at this stage to change significantly this.
Beside the comments (mostly formatting), could you also add a new redirect for the old page?
931aa14
to
6c00b6b
Compare
Thanks, redirect added! |
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.
Hi @ltalirz , thanks for the work! I tried to read it thoroughly and made a few comments, could you take a look and tell me what you think?
As an additional general one, could you change the -
itemization with *
? I know its super nitpicky but its what it says in the guidelines.
Hi @ramirezfranciscof thanks for checking the text; I've added a couple of your suggested fixed as well as fixed a broken reference. Let me know in case you think anything else is necessary - as mentioned above this is a rather specific developer-oriented documentation that will likely become obsolete to some degree once the REST API is touched for the next time. |
Ah, I see. Sorry if these were too much corrections then: please rebase the work and I'll merge it. Also, if this is the case, could I ask you to open an issue mentioning that the section is "temporary" and what other sections might be in the same bag? So that we remember in case people make comments to improve these or don't forget anything when the time comes to make the changes. |
Move the documentation on how to extend the AiiDA REST API to the "Internal Architecture" section. Co-authored-by: Giovanni Pizzi <gio.piz@gmail.com> Co-authored-by: ramirezfranciscof <ramirezfranciscof@users.noreply.github.com>
bee5e5a
to
18d5882
Compare
Done!
I think it's fine - we will remember when changes are made to the REST API. This piece of documentation has a very limited target group (developers who want to extend the REST API) - if anyone would like to invest time to improve it, they are welcome, but I think it is not a very important part of the docs (e.g. I started editing the REST API without ever reading it). |
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.
Ready to go!
fixes #4043
Move the documentation on how to extend the AiiDA REST API to the
"Internal Architecture" section.