-
Notifications
You must be signed in to change notification settings - Fork 31
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
Consider changing this to be a non-catkin independent package? #66
Comments
That makes sense to me, Mike. While I "maintain" this package in the minimal sense of merging pull requests and making new releases when needed, I don't normally do any development on it myself. Becoming catkin-independent might be easier to do and make more sense in the context of a ROS 2 version of the tool. Would that help any? I guess you'd still want it to work with ROS 1 messages, right? |
@tfoote @dirk-thomas Can I get your thoughts on this? Given the awkwardness of how the doc jobs currently install rosdoc_lite, I think there would be some advantage in moving it to be a independent package. I think there would be three possible ways forward:
Would you be in support of any of these paths? |
I don't consider cloning the default branch of Regarding the message generation: I don't think that In the very early design phase of the new build farm I proposed to let the doc jobs do the following instead (ros-infrastructure/jenkins_scripts#58):
My proposal was not considered to reduce the effort for transitioning from the old to the new build farm. I am still convinced that the best path forward would be to change the doc jobs to perform actual builds and then document the "result" (meaning the installed headers) as well as only the exported messages. |
Oh wow, so I'd seen the build workspace in isolation and install, but didn't realise the whole rewritten CMakelists thing. That's... quite a hack. Based on my own experiments, the epydoc generator is very dependent on having some kind of built workspace, since it has to be able to import the modules it is documenting. In any case, once you're only documenting the "exported" messages, then it matters a lot less how exactly you do it and you might as well just document from the python results rather than have to re-parse the I'm unsure if I'm on board with only documenting exported headers— it's reasonable to want to co-locate docs with the implementations in the cpp files (eg), and those doc blocks would be invisible to such a generator. The rosdoc_lite package is not a large or complicated affair; perhaps I'm overcomplicating things by trying to work within it, and it would make more sense to just start over and build something which does exactly what I want; there are a number of constraints that it and the buildfarm are subject to which I am not. |
I do consider cloning the default branch of I agree with Mike's notion of building his own documentation scripts. That's generally what people do when they need some feature that does not easily fit into the current implementation. |
I am willing to make fixups to rosdoc_lite to make it better suited to building docs on Clearpath's infrastructure; I believe the suggested improvements would allow ros_buildfarm's doc jobs to be simplified and improved, as well as improving the story for developers generating docs locally on their laptops. However, if the recommendation from upstream is that I implement these things on a proprietary fork, I can do so, and rosdoc_lite and ros_buildfarm will remain as they are. |
Nobody is suggesting to create a fork. I will comment with my opinion on the previous mentioned bullets:
Copying code into another location is imo not a good idea in general. If the existing code is not suitable to be reused in should be refactored to enable that. Otherwise if the
ROS distro agnostic packages are very problematic since they must continue to be supported for every ROS distribution. Even the ones which have been EOLed before since some users are still building them from source. Therefore as few code as possible is released that way. It would also make changing the code significantly more effort since it needs to maintain backward compatibility. That is the reason why currently
Imo the documentation generated from the
For a repository with active development I agree that tagging versions makes a lot of sense. I just doubt the benefit in this case (at least based on the past) since there is almost no activity in this repo. Only very few bug fixes are being applied and they are usually immediately "released" / rolled out to be used. If someone wants to tag this repo with a version number it would be trivial to update the build farm to use a specific tag instead of master. If I remember correctly we discussed this before and since there was no active development nothing was changed since it basically didn't make a real difference. If "enough" people raise their voice about the doc jobs (and their pseudo builds) it is more likely that I can bring up the problem in our team meeting again and get time allocated to work on it. Without I just can't spend any time on it due to other projects having priority. |
The generated Python messages contain the entire contents of the original Absent the issue with msgenator, are you opposed to the idea of the ros documentation tool being a pypi package rather than a distro package? |
The sphinx documentation of the generated Python code contains the content of the I am fine with it being a pure Python package. I am just not sure how you want to address the dependencies on |
There is confusion— I am not meaning to propose that the messages would be documented by sphinx or epydoc. There would still be a bespoke generator for messages, but it would use as its source the generated python modules (eg, to discover the fields, types, dependencies, etc). This breaks the dependency on the parsing logic in genmsg. |
I think I don't understand how the proposed process would look like. The generated code contains the |
Taking Imu for example, the generated doc has three parts:
The only piece missing is a reference to the exact filename in the package, though we don't really have that today either (it would be cool if it linked right back to the file on github, at the appropriate branch). Perhaps this is marginally more fragile than using the original parser, but as has been pointed out, neither has changed in years, and although the fields are "private" in the sense that they're prefixed with underscores, in reality they're part of the stable API and used all over the place, eg: https://github.com/ros/ros_comm/search?q=_slot_types+OR+_full_text To further simplify, another way could be to do as catkin itself does and generate rst pages for Sphinx to process— that gets rosdoc_lite completely out of the business of slinging HTML around. As far as the doxygen dependency, I don't have a good answer for that— I think the simplest thing would be for (the proposed pypi-based) rosdoc_lite to simply check for doxygen with a trivial invocation, and when missing inform the user that doxygen invocations are skipped until it is installed. The stdeb config that produces |
Thanks for clarifying. The proposed process sounds reasonable to me. (In my personal opinion using the ROS msg files instead of the generated Python code sounds better but I am ok with |
The only catkin-y thing this package has is a dependence on genmsg, which it uses to generate message documentation, in this one file:
https://github.com/ros-infrastructure/rosdoc_lite/blob/d6f63f94c62b3b73f5d3350a7773ffb7478b353a/src/rosdoc_lite/msgenator.py
It feels awkward to me that rosdoc_lite has to be "built" in a catkin workspace, in order to document another catkin workspace— seems like it would be cleaner for it to be a fully external affair, etc,
sudo apt-get install python-rosdoc; rosdoc path/to/my/package
I don't have a ton of bandwidth to look into this further, but I'm curious to have others' thoughts on it.
The text was updated successfully, but these errors were encountered: