-
Notifications
You must be signed in to change notification settings - Fork 77
Issue 525 - Added doxygen-based documentation #526
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: master
Are you sure you want to change the base?
Conversation
Added documentation generated by 'doxygen'. It requires both 'doxygen' and 'graphviz'. This commit is entirely POC - it demonstrates the documentation that can be generated with no modifications to the existing code but we should definitely provide some additional information in both the source files and overviews.
Sample output - I attempted to attach a zip file containing the HTML content but it was far too large due to the bundled images. (Call graphs, etc.) Therefore I pushed it to my own branch - we'll want to make sure that it's not merged later. Unfortunately it looks like I can't provide a 'live' link to it - github insists on showing the contents. |
Thanks! I'll take a closer look when I get home from pgconf. Do you have GitHub Pages enabled on your repo? If you wanted, you could put a live preview there. Are you following the PL/Java refactoring in PR #399? The goal is to get much more of the implementation into Java and for quite a lot of the legacy C code to (thereafter) disappear. So whatever doxygen can generate for free is nice, but there may be limited value in adding a lot of doc comments in C code that should go away. I would certainly favor a Java-centric FDW implementation too. |
I wasn't aware of that effort. Thanks for the info. Is it C or C++? I don't recall if the server can directly link to C++ yet but it's often possible to use it as long as you're careful to use the 'extern C' on the bits that connect. I've been looking at some recent C++ code and it's very different from what I used in the past... but to be fair so is a lot of the C I'm seeing in different projects! I'll update the PR this evening or tomorrow - biggest changes are some initial groups and dropping all of the 'static' stuff since it looks like there's still something left. Maybe not a lot but I think functions can be added on an individual basis. As for 'futility'....Perhaps - but I think there's a lot to be said for keeping at least some of this in an attic since it's a well-proven demonstration of working with both JNI and database internals. That said I also know that JNA is also well-proven and a lot easier to implement than JNI. I don't know if there would be a performance hit in this case, but it's possible that the "best" solution may be to a mixture of JNI and JNA where the latter is limited to wrapping the existing libpq library. As for the FDW....I gave that a bit more thought yesterday and I think a FDW might always require a stub but I'm not certain yet since it looks like it may require nothing more than providing the name of the FDW. If that's the case then you would expect there to be a dynamic solution, or perhaps a solution already available in PGXN. I also forgot to mention a second method, for validation. It's optional but can be used to validate the parameters passed to both the FDW and Server. This validation can run pretty deep - it's not limited to just verifying that it's a well-formed string. |
It also was mentioned in #430. #430 might be the ideal place to continue FDW conversation.
All of PL/Java's native code is currently C. If you might like writing C++ for PostgreSQL, check out Yurii's talk on
That's the great thing about git—nothing is ever gone. If it helps to prioritize where it's worth adding documentation, certainly
More on this at #399.
More on #430 when I have a bit of time. |
Added documentation generated by 'doxygen'. It requires both 'doxygen' and 'graphviz'.
This commit is entirely POC - it demonstrates the documentation that can be generated with no modifications to the existing code but we should definitely provide some additional information in both the source files and overviews.