-
Notifications
You must be signed in to change notification settings - Fork 495
[NFC] Update doxygen build and configuration files #5718
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
base: main
Are you sure you want to change the base?
Conversation
I know that doxygen isn't everyone's favorite thing, but I've found it useful, but it hasn't been updated in a bit. This PR updates it and adds a just ds (doxygen-serve) command to easily serve the generated docs for local browsing. The doxygen build is added because doxygen needs to be built with the correct clang version to avoid warnings and errors.
| the system LLVM/clang installation to enable clang-assisted parsing of C++ code. | ||
| """ | ||
|
|
||
| def _doxygen_repository_impl(ctx): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wot in tarnation... is there really no easier way to build Doxygen from Bazel?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would https://github.com/TendTo/rules_doxygen be suitable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no idea. I didn't try lol. I really dislike dealing with build tooling so went with the easiest thing that could possibly work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's wait to see if Felix agrees, but I'd suggest asking Claude to use rules_doxygen. If it works, there'll be a lot less code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well... if we're going for the easiest thing then doing bazel_dep(name = "rules_doxygen", version = "2.6.1") and following the Use instructions is a lot easier here than this approach. Sometimes we don't need to reinvent the wheel, even if we have automated the reinventing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another vote for rules_doxygen then
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had tried that approach initially but kept running into issues with the clang support... building the doxygen here was the only way I was able to get it to work reasonably with a couple of the other updates.
|
moving this to draft. there's a few bits to fix up but it's a low priority, will work on it as time allows. |
I know that doxygen isn't everyone's favorite thing, but I've found it useful, but it hasn't been updated in a bit. This PR updates it and adds a just ds (doxygen-serve) command to easily serve the generated docs for local browsing.
The doxygen build is added because doxygen needs to be built with the correct clang version to avoid warnings and errors.
I had claude do the bazel bits because I really hate dealing with build systems.