Skip to content

Conversation

@NeoZhangJianyu
Copy link
Collaborator

  1. correct the requirements.txt
  2. add script to build online doc by one click.

Copy link
Collaborator

@yinghu5 yinghu5 left a comment

Choose a reason for hiding this comment

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

let's try

@yinghu5 yinghu5 merged commit 7c6a904 into opea-project:main Oct 11, 2024
Copy link
Collaborator

@mkbhanda mkbhanda left a comment

Choose a reason for hiding this comment

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

Minor comments
line 36: echo "repo ${REPO} exists, skipping cloning"
line 57: echo "CP html to ${PUBLISHDIR}" .. better to write "Copied html content to $(PUBLISHDIR}"

LGTM

BUILDDIR=_build
PUBLISHDIR=${RELEASE_FOLDER}/latest

cp -r ${BUILDDIR}/html/* ${PUBLISHDIR}
Copy link
Contributor

@dbkinder dbkinder Oct 11, 2024

Choose a reason for hiding this comment

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

@NeoZhangJianyu @mkbhanda @yinghu5 @chensuyue @intelkevinputnam You left out a step that's in the "make publish" steps that handles the case when a file was deleted from the previously published documents. You should remove everything from the PUBLISHDIR folder before copying all the files that were just built:

rm -rf $(PUBLISHDIR)/*

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I will update as your comment later.

Thank you!

@@ -0,0 +1,28 @@
#!/bin/bash

sudo apt install git doxygen graphviz -y
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you @dbkinder for continuing to be the doc guardian angle for OPEA!!
https://doxygen.nl/index.html Doxygen "automates the generation of documentation from source code comments, parsing information about classes, functions, and variables to produce output in formats like HTML and PDF" but I see no signs of its use in our various OPEA subprojects. Just see references to it in opea-projects/docs in scripts/setup_env.sh and scripts/show-versions.py. For that matter I do not see graphviz either. Are these being included in anticipation of use @NeoZhangJianyu ?

Copy link
Collaborator Author

@NeoZhangJianyu NeoZhangJianyu Oct 14, 2024

Choose a reason for hiding this comment

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

Doxygen is not used in fact. It's copied from another script. I will remove it.

graphviz is used by OPEA doc build, refer to https://github.com/opea-project/docs/blob/main/developer-guides/docbuild.rst:

"
For Ubuntu, use:

sudo apt install python3-pip python3-wheel make graphviz

"

@NeoZhangJianyu
Copy link
Collaborator Author

Minor comments line 36: echo "repo ${REPO} exists, skipping cloning" line 57: echo "CP html to P U B L I S H D I R " . . b e t t e r t o w r i t e " C o p i e d h t m l c o n t e n t t o (PUBLISHDIR}"

LGTM

Yes, I will update as your comment later.

Thank you!

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.

5 participants