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

[feature] Pull docs from OpenWISP modules to create a unified documentation Website #107 #189

Open
wants to merge 65 commits into
base: master
Choose a base branch
from

Conversation

pandafy
Copy link
Member

@pandafy pandafy commented Feb 22, 2024

Closes #107

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

We can publish this on GC in the __new__ subdirectory.

@pandafy pandafy force-pushed the issues/107-comprehensive-docs branch 5 times, most recently from b8a28f2 to 4261084 Compare April 26, 2024 19:36
<!DOCTYPE html>
<html>
<head>
<meta http-equiv="refresh" content="0; URL='/22.05/index.html'" />
Copy link
Member

Choose a reason for hiding this comment

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

This looks fragile: what happens when we release a new stable version?
Is it possible to have an alias for the latest stable version?

Copy link
Member Author

Choose a reason for hiding this comment

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

I couldn't find a way to render an HTML file with context from sphinx configuration.

config.yml Outdated
- name: openwisp-users
branch: reorder-docs
- name: openwisp-utils
branch: reorder-docs
Copy link
Member

Choose a reason for hiding this comment

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

Can we change the implementation so that the order of this list is respected and the index here reflects this?

Screenshot from 2024-04-29 21-26-20

Without this change I am not sure we have a way to decide the order of these documents, which is problematic.

Copy link
Member Author

Choose a reason for hiding this comment

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

Instead of using modules/*/docs/* in the TOC tree. We can specify the path of each module to get the desired order.

index.rst Outdated
:glob:

user/quickstart
modules/*/docs/*
Copy link
Member

Choose a reason for hiding this comment

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

maybe we need to name the folders with numbers to order them as we want? Would that impact the URLs?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that would change the URLs.

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

Please rebase on the latest master and see my comments below.

pulling_modules.py Outdated Show resolved Hide resolved
run-qa-checks Outdated Show resolved Hide resolved
pulling_modules.py Outdated Show resolved Hide resolved
reference.rst Show resolved Hide resolved
OpenWISP Contributor's Board automation moved this from To do (general) to In progress May 8, 2024
@pandafy pandafy force-pushed the issues/107-comprehensive-docs branch 4 times, most recently from 8ab2e18 to acc4a16 Compare May 10, 2024 17:33
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

I am happy of the progress we are making here! 👏 🔥 🚀

Here's the summary of the improvements we have discussed.

Stable alias

We need to generate a symbolic link "stable", pointing to the latest available stable version.

We can generate it only if the following conditions are true:

  • The link doesn't exist
  • Or it exists and point to a different version that what we have computed to be the latest.

To compute the latest version, instead of having to specify it manually, let's determine that automatically.
The stable version is the highest non dev version, we can compute it by getting all available versions, and getting the higher value (eg: use max(version_list)).

YAML: avoid repeating modules

Let's avoid having to repeat the available modules in each version, eg:

---

modules:
  - name: openwisp-controller
    dir_name: controller
  - name: openwisp-monitoring
    dir_name: monitoring
  - name: openwisp-firmware-upgrader
    dir_name: firwmare-upgrader
  - name: openwisp-notifications
    dir_name: notifications
  - name: openwisp-network-topology
    dir_name: network-topology
  - name: openwisp-users
    dir_name: users
  - name: openwisp-utils
    dir_name: utils
versions:
  # just for example purposes
  # you can see the branch is missing
  # because it's implicit, we don't need to specify it
  - name: "24.06"
  - name: "22.05"
  - name: dev
    branch: reorder-docs

Menu is shown twice

The menu is shown twice, we need to have 1 menu only.

Separate current 22.05 docs from dev

We need to use the 22.05 branch for stable docs, so that we can do any change we need in the master branch without creating conflicts or missing pages with the old stable docs.

Simplify URL structure

With the current structure we would end up with an URL like the following:

https://openwisp.io/docs/stable/modules/openwisp-controller/docs/user/templates.html.

The goal is to get more friendly URLs like:

https://openwisp.io/docs/stable/controller/user/templates.html.

Here's a possible solution I thought, which as a bonus also allows to avoid building files like README.rst, CONTRIBUTING.rst, CHANGES.rst which we should not build.

Every time we clone/checkout, we copy or link the docs/ directory to the root of the docs repository, renaminig it to the value of the dir_name attribute in the YAML.

We have to instruct sphinx to not build anything in the modules directory, the following seems to work for me:

diff --git a/conf.py b/conf.py
index e711b80..dd19d8c 100644
--- a/conf.py
+++ b/conf.py
@@ -85,7 +85,7 @@ language = 'en'
 # List of patterns, relative to source directory, that match files and
 # directories to ignore when looking for source files.
 # This patterns also effect to html_static_path and html_extra_path
-exclude_patterns = ['README.rst', '_build', 'Thumbs.db', '.DS_Store']
+exclude_patterns = ['README.rst', '_build', 'Thumbs.db', '.DS_Store', 'modules/*']
 
 # The reST default role (used for this markup: `text`) to use for all
 # documents.

Once the build step is completed, we can remove the copies/link.

We need to add those dirs in the .gitignore so we won't commit anything by mistake.

Allow building only a specific version

The previous step will make building only a specific version harder, due to the copying/linking renaming and deleting of the docs directory each time.

Can you add an option to allow do that through the Makefile?

Include more modules

Include the following repos:

  • IPAM
  • openwisp-config (let's use openwrt-config-agent as dir_name)
  • openwrt-openwisp-monitoring (let's use openwrt-monitoring-agent as dir_name)
  • wifi-login-pages
  • ansible-openwisp2
  • docker-openwisp

build.py Outdated
Clone or update a repository based on the module name and branch provided.
If the repository already exists, update it. Otherwise, clone the repository.
"""
repo_url = f'https://github.com/openwisp/{module_name}.git'
Copy link
Member

Choose a reason for hiding this comment

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

I was thinking here, since our repos are open source and public, I think we can use the git protocol here instead of SSH, isnt't it?

This will make it easier for us to work on the entire set of documents from one place and push changes to all the modules more easily. Now I found myself having to use git remote set-url origin ... a lot before being able to push.

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

While reviewing and improving the docs of OpenWISP Controller, I found quite a lot of broken links.
Our build here is supposed to be able to detect broken links, but I think the changes of this PR are making the run-qa-checks script ineffective, so please double check that, ensure that broken links are detected.

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

Another thing we will need to verify is that PDF opens correctly.
A few days ago I couldn't open the generated PDF, now the error went away.
However, it's better to have an automatic verification mechanism.

@pandafy pandafy force-pushed the issues/107-comprehensive-docs branch 8 times, most recently from bd32d52 to a38bec6 Compare May 20, 2024 12:54
@pandafy pandafy force-pushed the issues/107-comprehensive-docs branch from 32423d3 to 5cc0e78 Compare June 6, 2024 19:32


class OpenwispDummyBuilder(DummyBuilder):
name = 'ow_dummy'
Copy link
Member

Choose a reason for hiding this comment

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

can you add a docstring explaining what this class does?
Can we find a better name for this? Like VersionSwitcherIndexBuilder or anything similar?

Makefile Outdated
@@ -240,3 +257,7 @@ dummy:
$(SPHINXBUILD) -b dummy $(ALLSPHINXOPTS) $(BUILDDIR)/dummy
@echo
@echo "Build finished. Dummy builder generates no files."

.PHONY: ow_dummy
ow_dummy:
Copy link
Member

Choose a reason for hiding this comment

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

same here, can we find a more descriptive name?

Makefile Outdated

.PHONY: ow_dummy
ow_dummy:
$(SPHINXBUILD) -b ow_dummy $(ALLSPHINXOPTS) $(BUILDDIR)/ow_dummy
Copy link
Member

Choose a reason for hiding this comment

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

same for $(BUILDDIR)/ow_dummy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

[docs] Comprehensive/Unified Documentation Website
2 participants