MDEV-39806: Add a per-push/per-pull-request checker for Markdown API docs - #5153
MDEV-39806: Add a per-push/per-pull-request checker for Markdown API docs#5153gkodinov wants to merge 1 commit into
Conversation
This comment was marked as low quality.
This comment was marked as low quality.
c0b255a to
538d427
Compare
vuvova
left a comment
vuvova
left a comment
There was a problem hiding this comment.
I never created or maintained github actions so I cannot really review that.
Commands under run look ok (if I'm not going to nitpick)
| @@ -0,0 +1,50 @@ | |||
| name: Generate Plugin API Docs | |||
|
|
|||
| on: | |||
There was a problem hiding this comment.
If you want this workflow to run on other branches and on Pull Requests that are not targeting main
then it is not enough to commit it to the main branch.
Every supported branch should have it.
Going one step back,
why you want this workflow to run for every push or bb-* branch?
This is a lot of stress imposed on public GitHub runners; where's the added value? Because I see no mechanism where the results from a push are retrieved/analyzed.
| - 'bb-*' | ||
| pull_request: | ||
| branches: | ||
| - 'main' |
There was a problem hiding this comment.
This will only trigger if someone opens a Pull Request towards main or a bb-* branch.
What is your intention here?
There was a problem hiding this comment.
I believe this should only be in the main branch.
| git submodule update --init --recursive libmariadb | ||
| mkdir bld | ||
| cd bld | ||
| cmake -DENABLE_GCOV=OFF \ |
There was a problem hiding this comment.
You could've probably used -DWITHOUT_SERVER=ON to skip most of the cmake checks.
There was a problem hiding this comment.
This was done prior to having the shell script. I was hoping to get the shell script PR reviewed and merged first. But, oh, well. Rejigled with the idea to have the shell script.
RazvanLiviuVarzaru
left a comment
RazvanLiviuVarzaru
left a comment
There was a problem hiding this comment.
As I remember from our previous discussions,
the main idea was that in the server repository to only have a Pull Request check action, that will fail for any documentation errors. assuming that you configured the tool to fail the action when there's a legitimate error on docs.
Did I missed something in the meantime?
vuvova
left a comment
vuvova
left a comment
There was a problem hiding this comment.
agree with @RazvanLiviuVarzaru, my recollection also is that we agreed that cmake and other build prerequisites aren't required to generated documentation
538d427 to
d185611
Compare
I was hoping to get #5112 merged first, but it seems like we'll be doing the two in parallel. OK. I've updated the PR with code that's based on the files in #5112. |
|
Note that we will need to also update the moxygen version in mariadb-doc-gen: there are some fixes in it that are needed: https://github.com/sourcey/moxygen/releases/tag/v2.1.12 |
…docs Create a GitHub action into the MariaDB server tree to build the Markdown documentation using the cmake target provided by MDEV-39718. This workflow generates API documentation for the plugin using a Docker container packed with all the extra doxygen/moxygen tools.
d185611 to
141e17e
Compare
3 participants
Create a GitHub action into the MariaDB server tree to build the Markdown documentation using the cmake target provided by MDEV-39718.
This workflow generates API documentation for the plugin using a Docker container packed with all the extra doxygen/moxygen tools and the latest CMake dependencies.