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

Add new OCS API documentation #10006

Merged
merged 1 commit into from
Jun 8, 2023
Merged

Add new OCS API documentation #10006

merged 1 commit into from
Jun 8, 2023

Conversation

provokateurin
Copy link
Member

I wanted to link to the static file directly from the TOC, but it's not possible :/
Please don't merge yet, as the spec itself is not finalized.

Makefile Show resolved Hide resolved
Makefile Show resolved Hide resolved
Copy link
Member

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

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

I noticed a issue during build locally, but otherwise this looks really nice 🎉

Main blocker from my perspective:
https://github.com/nextcloud/documentation/pull/10006/files#r1205108612

Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Only checked the translation thing.

Maybe we can just keep the "old docs" in parallel until the transition of all the available information to the openapi spec was done?

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
developer_manual/_static/openapi.html Outdated Show resolved Hide resolved
build/generate-openapi.sh Outdated Show resolved Hide resolved
developer_manual/client_apis/OCS/ocs-translation-api.rst Outdated Show resolved Hide resolved
developer_manual/client_apis/OCS/ocs-translation-api.rst Outdated Show resolved Hide resolved
developer_manual/client_apis/OCS/ocs-translation-api.rst Outdated Show resolved Hide resolved
developer_manual/client_apis/OCS/ocs-translation-api.rst Outdated Show resolved Hide resolved
developer_manual/client_apis/OCS/ocs-translation-api.rst Outdated Show resolved Hide resolved
developer_manual/client_apis/OCS/ocs-translation-api.rst Outdated Show resolved Hide resolved
@provokateurin provokateurin force-pushed the feat/openapi branch 2 times, most recently from c984438 to db696c6 Compare May 25, 2023 15:21
@nickvergessen
Copy link
Member

For me it does not load:

Firefox

Uncaught SyntaxError: illegal character U+20AC
[stoplight-elements.js:2:422677](http://localhost:63342/documentation/developer_manual/_build/html/com/_static/stoplight-elements.js)

Chrome

caught SyntaxError: Invalid or unexpected token

@provokateurin
Copy link
Member Author

How are you loading it? I think it's a problem with the web server.
Using python -m http.server I also get this error, but using https://www.npmjs.com/package/serve works.

@juliusknorr
Copy link
Member

How are you loading it? I think it's a problem with the web server. Using python -m http.server I also get this error, but using npmjs.com/package/serve works.

Also worked fine for me with php -S 0.0.0.0:8081

@provokateurin
Copy link
Member Author

@nickvergessen Is there something still missing?

@come-nc
Copy link
Contributor

come-nc commented Jun 5, 2023

I’m failing to build this:

Updating dependencies
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Root composer.json requires phpdocumentor/reflection dev-php7.0 -> satisfiable by phpdocumentor/reflection[dev-php7.0].
    - phpdocumentor/reflection dev-php7.0 requires php ^7.0 -> your php version (8.1.2) does not satisfy that requirement.
  Problem 2
    - Root composer.json requires leafo/scssphp dev-master -> satisfiable by leafo/scssphp[dev-master].
    - leafo/scssphp dev-master requires php ^5.4.0 || ^7 -> your php version (8.1.2) does not satisfy that requirement.
  Problem 3
    - phpdocumentor/reflection-docblock 4.3.0 requires php ^7.0 -> your php version (8.1.2) does not satisfy that requirement.
    - juliushaertl/phpdoc-to-rst dev-php7.0 requires phpdocumentor/reflection-docblock 4.3.0@stable -> satisfiable by phpdocumentor/reflection-docblock[4.3.0].
    - Root composer.json requires juliushaertl/phpdoc-to-rst dev-php7.0 -> satisfiable by juliushaertl/phpdoc-to-rst[dev-php7.0].

make: *** [Makefile:45 : icons-docs] Erreur 2

@provokateurin
Copy link
Member Author

That's the icons, they are broken since a long time. Unrelated to this change. You can comment the icons-docs target in the Makefile to skip it.

@come-nc
Copy link
Contributor

come-nc commented Jun 5, 2023

I tried instead of make developer-manual-html to do make openapi-spec, then I cd into developer_manual and run make SPHINXBUILD=sphinx-autobuild html, but still the link to http://127.0.0.1:8000/_static/openapi.html gives a blank page.

Web console shows:

stoplight-elements.js:2 Uncaught SyntaxError: Invalid or unexpected token
favicon.ico:1 GET http://127.0.0.1:8000/favicon.ico 404 (Not Found)

@provokateurin
Copy link
Member Author

See comments above. Try @juliushaertl's way to serve the documentation.

@come-nc
Copy link
Contributor

come-nc commented Jun 5, 2023

I tried php -S 0.0.0.0:8081 but in the main folder I only get the index with links poiting to docs.nextcloud.com, in the developer_manual folder I get The requested resource / was not found on this server. When opening the web page.

make SPHINXBUILD=sphinx-autobuild html is the documented way if that does not work anymore the README needs to be updated and explain how to test the documentation locally.

@juliusknorr
Copy link
Member

make SPHINXBUILD=sphinx-autobuild html currently only works within the individual documentation folders developer_manual, user_manual, admin_manual. As those are separate sphinx projects I don't see a way to use the autobuild tooling.

While not a nice experience it is documented as

  1. Enter the documentation section cd user_manual

However the document sections Makefile doesn't cover the developer documentation pre-build steps which are in the main root makefile.

Steps for testing manually:

  • run make developer-manual-html in the root
  • run php -S 0.0.0.0:8081 in developer_manual/_build/html/com/

I've pushed two commits to unblock this PR, but it would be nice to follow up with a cleaner solution so that either way of building involves the required build steps.

@juliusknorr juliusknorr requested a review from come-nc June 6, 2023 13:25
@provokateurin
Copy link
Member Author

I disabled the icons-docs in an earlier version of this PR, but @nickvergessen didn't want to have this. I agree with both sides, but prefer to not touch it in this PR. Better to have a separate PR that either fixes it in a correct way or disables/removes it.

@come-nc
Copy link
Contributor

come-nc commented Jun 8, 2023

Good enough for me, at least I could test it and it works.

Merge?

@provokateurin
Copy link
Member Author

I think @nickvergessen's concerns have been all addressed, but I don't really want to merge @juliushaertl commits in this PR. @juliushaertl could you make a separate PR? I'll happily approve that, but the commits really don't belong here.

@juliusknorr
Copy link
Member

Removed the commits and pushed them to #10584 as they were based on this one.

@juliusknorr juliusknorr merged commit 39e4925 into master Jun 8, 2023
@juliusknorr juliusknorr deleted the feat/openapi branch June 8, 2023 19:13
@backportbot-nextcloud
Copy link

The backport to stable27 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable27
git pull origin stable27

# Create the new backport branch
git checkout -b fix/foo-stable27

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable27

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

@provokateurin
Copy link
Member Author

Thanks! ❤️

@provokateurin
Copy link
Member Author

Meh, the problem @nickvergessen and @come-nc had with the illegal character thing also happen on the deployed version. Not sure what is going on there, I will investigate it (seems like some character set problem?)

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.

5 participants