MDEV-39718: Produce Markdown plugin API documentation#5112
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new CMake utility to generate plugin API documentation using Doxygen and Moxygen, along with various documentation formatting improvements in header files to ensure proper rendering. The review identifies a critical bug in the CMake macro where the first source file is inadvertently removed from the list, causing incomplete documentation. Additionally, the feedback suggests using functions instead of macros for better encapsulation, adopting more idiomatic boolean options in CMake, and defaulting the documentation generation to off to avoid build-time dependency issues in environments lacking the required tools.
e20f707 to
436a2ca
Compare
There was a problem hiding this comment.
Hi, @gkodinov ,
I don't think you need to integrate this as part of cmake, it looks over-engineered for the purpose,
when you can bundle every tool you need in a Docker container and run it as part of GitHub Actions.
Not to say, every change for this cmake file is a server build,
whether, having a containerized build environment means you can control
the behavior of "what needs to be done" somewhere else.
I will re-iterate what I told you on the e-mail so @vaintroub can read also.
Now that I read this patch, you don't even need cmake in container with what I proposed, simplifying the Dockerfile by a lot.
-------- from e-mail ------------
_From what you describe, you absolutely do not need Buildbot or a build host.
You only need GitHub Actions and a container with the required tools.
First of all, for local testing, you can build a container image that includes the tools you need.
For example, at the end of this email, you will find a simple Dockerfile with the tools you've requested (and some build-tools required by a server CMake invocation).
The generation logic (CMake, calling Doxygen, Moxygen) can be included in a script
as part of this containerized build environment (in my example, it is generate.sh). You can also copy the Doxygen configuration file, Doxyfile.in, into this container.
Then, you can publish this container to the GitHub Container Registry.
and later use it as part of a GitHub Action that responds to push events on the branches you are interested in. GitHub can run containers on its public agents.
Any further logic, such as opening a Pull Request or performing a regression test can be accommodated as part of this GitHub Action._
FROM node:20-bookworm-slim
ARG DEBIAN_FRONTEND=noninteractive
ARG MOXYGEN_PACKAGE=moxygen@0.8.0
RUN apt-get update \
&& apt-get install -y --no-install-recommends \
bison \
build-essential \
ca-certificates \
cmake \
doxygen \
flex \
gettext-base \
git \
graphviz \
libaio-dev \
libfmt-dev \
liblz4-dev \
liblzma-dev \
libncurses-dev \
libnuma-dev \
libpcre2-dev \
libreadline-dev \
libsnappy-dev \
libssl-dev \
libsystemd-dev \
ninja-build \
perl \
pkg-config \
zlib1g-dev \
&& rm -rf /var/lib/apt/lists/*
RUN npm install --global "${MOXYGEN_PACKAGE}" \
&& npm cache clean --force
WORKDIR /work
COPY Doxyfile.in /opt/plugins-api-docs/Doxyfile.in
COPY generate.sh /usr/local/bin/generate-plugins-api-docs
RUN chmod +x /usr/local/bin/generate-plugins-api-docs
ENTRYPOINT ["generate-plugins-api-docs"]
There was a problem hiding this comment.
The idea behind pushing as much of it as possible into cmake is that developers should be able to efficiently drive this via normal commits to the repo. Besides, developing the API docs is easier if you just hit "make" when you need a preview.
There was a problem hiding this comment.
@RazvanLiviuVarzaru ghcr.io docker recipes are scoped to either a login or to an org. I would really appreciate if you could add the above docker image to https://github.com/orgs/MariaDB/packages. Just remove the things after (and including) WORKDIR.
And please do not use @0.8.0 in the recipe: just install whatever's latest.
I can do the rest from the relevant github action script.
There was a problem hiding this comment.
I also wonder if the dependencies can be added to bb-worker? would that be too much?
There was a problem hiding this comment.
@gkodinov Hi,
The sample Dockerfile it was just an example for you to get an idea of how it could look like.
If you are going with the solution I proposed, you can build it to your like and push it yourself to GHCR.
There was a problem hiding this comment.
"I also wonder if the dependencies can be added to bb-worker? would that be too much?"
I am not onboard with having Buildbot producing documentation.
Buildbot is concerned with building and testing the server. The helper tools, i.e. producing documentation for the knowledge base can be hosted elsewhere. Generating documentation for the plugin API naturally happens at a lower cadence and frequency than the server’s need to be built and tested (which is for every commit). That is why I suggested using GitHub Actions.
There is no need for Buildbot to invest resources so that, for every commit, a builder generates documentation that people will most likely look at very rarely, if at all, except perhaps for a small number of people. Consequently, I do not plan to add the tools to a bb-worker.
There was a problem hiding this comment.
"The idea behind pushing as much of it as possible into cmake is that developers should be able to efficiently drive this via normal commits to the repo. Besides, developing the API docs is easier if you just hit "make" when you need a preview."
It somehow feels like implementing a solution is taking priority over validating the output of such a solution with the people who will utilize it.
I would have expected you to generate some documentation with the tool first and discuss it internally with the developers, so that you could agree on a format and process. Did that happened?
What is a developer supposed to do if they run CMake to generate documentation?
Will they open a pull request to update the knowledge base? Is that their responsibility?
If there is no feedback loop where the documentation produced as a result of the developer’s code contributions is visible, then I do not see the point in having them generate it via cmake.
I tried using Doxygen and Moxygen in the form you proposed, and I found the documentation rather cumbersome to read. Is it just me?
There was a problem hiding this comment.
It's under review in this PR. But yes, I have had discussions prior to the PR.
Implemented a cmake utility macro to generate markdown documentation. Generated the plugin API headers. Fixed some doxygen comment mistakes in these.
Implemented a cmake utility macro to generate markdown documentation.
Generated the plugin API headers.
Fixed some doxygen comment mistakes in these.