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

Remove apidocs completely #1808

Merged
merged 1 commit into from May 12, 2021
Merged

Conversation

lzap
Copy link
Member

@lzap lzap commented Apr 8, 2021

This removes whole /api directory, also it updates the URL in the main menu and also drops unused api_v2 layout template.

@lzap
Copy link
Member Author

lzap commented Apr 8, 2021

For the record, rewrite rules PR is here: theforeman/foreman-infra#1600

@lzap
Copy link
Member Author

lzap commented Apr 12, 2021

For the record, the discussion is at https://community.theforeman.org/t/proposal-move-apidocs-from-git-to-infra-web/23059

The infra (redirects) change is at: theforeman/foreman-infra#1600

For easier review, the only two changes at the moment are:

  • Menu URL change in _config.yaml
  • Removal of the layouts/api_v2.html
  • Removal of the whole /api/ directories

You also want to review this github repo which hosts the apidocs.theforeman.org domain prior merging this: https://github.com/theforeman/apidocs

@lzap
Copy link
Member Author

lzap commented May 4, 2021

For easier review I can remove deletion of "apidocs" directory, there is a change in layout. Anyone? @melcorr ?

@melcorr
Copy link
Member

melcorr commented May 4, 2021

I have no objections to this. Is everyone else OK with this?

@tbrisker
Copy link
Member

tbrisker commented May 4, 2021

While i'd be happy to get rid of this, It doesn't feel like the new api website is quite ready.
A few issues I noticed:

  • jquery isn't linked correctly, causing some interactivity to be broken (for example, click on "architectures" header and then on one of the actions, instead of expanding to show the action nothing happens)
  • the TEMPLATE directory shouldn't be displayed on the index
  • clicking on a version in the main index points to the index.html in that version which redirects to the v2.html index after a couple of seconds, instead of linking directly to v2.html.
  • the infra redirects still haven't been merged

@lzap
Copy link
Member Author

lzap commented May 4, 2021

  • jquery isn't linked correctly, causing some interactivity to be broken (for example, click on "architectures" header and then on one of the actions, instead of expanding to show the action nothing happens)

Fixed.

  • the TEMPLATE directory shouldn't be displayed on the index

Fixed in theforeman/apidocs@c104149

  • clicking on a version in the main index points to the index.html in that version which redirects to the v2.html index after a couple of seconds, instead of linking directly to v2.html.

This PR actually fixes this:

      - title: API docs
        url: https://apidocs.theforeman.org/foreman/latest/apidoc/v2.html
  • the infra redirects still haven't been merged

Let me check if there are some objections.

@lzap
Copy link
Member Author

lzap commented May 4, 2021

  • clicking on a version in the main index

So the concern was that from the index page (generated by the tree tool) its redirect.

Explanation: I do not want to create another place we would need to manually maintain, our main site is the place that will link everything. The generated index is just for web spiders, and those who would manually enter the address. But if you insist I can create some landing site, but we would need to edit it on each release and also we will never link it from the main site.

I asked in the infra PR for green light.

@lzap
Copy link
Member Author

lzap commented May 4, 2021

After a short IRC chat, I've updated the index.html to be more friendly in theforeman/apidocs@cceb426

@lzap
Copy link
Member Author

lzap commented May 4, 2021

Fixed also an extra LI HTML tag (bullet), if you can give this a review so we are all set and can wait for the infra PR merge. Thanks for the review!

@ekohl
Copy link
Member

ekohl commented May 4, 2021

When viewing a page I miss what version it's for. I'm leaning to keeping it simple and fall back to classic HTML frames. Thoughts?

@lzap
Copy link
Member Author

lzap commented May 6, 2021

I am not sure what exactly are you missing, I do not see any version on the current site:

https://www.theforeman.org/api/2.4/index.html

Compare to:

https://apidocs.theforeman.org/foreman/2.4/apidoc/v2.html

The version is in the URL itself, if you expect anything on the apidocs pages, I think we should update apidocs itself.

@ekohl
Copy link
Member

ekohl commented May 6, 2021

You're right that the current location also has no indication which version you're browsing. Not a blocker to merge

@lzap
Copy link
Member Author

lzap commented May 12, 2021

I believe I solved all the concerns, @tbrisker, I updated apidocs index page too. Can we merge this before 2.5 apidoc branching? This hasn't been done yet. FYI @upadhyeammit

I rebased but there were no changes, I wanted to make sure it applies cleanly and everything is up to date.

@tbrisker
Copy link
Member

GH won't let me approve because of the size of this change, but let's do this, if anything got missed we can follow-up on it. Great cleanup @lzap !

@tbrisker tbrisker merged commit 2efe92a into theforeman:gh-pages May 12, 2021
@lzap lzap deleted the remove-apidocs branch May 12, 2021 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants