Closed jcouball closed 1 month ago
Below is the best I can come up with.
I don't like how I have to inherit from Data.define. Firstly, it produces a Style/DataInheritance Rubocop violation. The reason this is a Rubocop violation is that an extra class is introduced into the inheritance chain vs. using Milestone = Data.define(...)
.
YARD also gives the following warning:
[warn]: in YARD::Handlers::Ruby::ClassHandler: Undocumentable superclass (class was added without superclass)
Additionally, to be complete, I would need to define the initializers and other interesting methods provided by the class returned by Data.define.
I am posting this issue to see there there is a better way to document Data objects.
# rubocop:disable Style/DataInheritance
# Defines a project milestone
#
# @api public
#
# @!attribute [r] id
# The unique milestone identifier (which is a URL)
# @example
# milestone.id #=> "https://example.com/db/btkkfcemc?a=dr&rid=197"
# @return [String]
#
# @!attribute [r] summary
# A short, one-line milestone summary
#
# A summary is never blank.
#
# @example
# milestone.summary #=> "Doc Storage Requirements Complete"
# @return [String]
# @api public
#
# @!attribute [r] description
# A possibly multi-line / multi-paragraph description of the milestone
#
# May be blank.
#
# @example
# milestone.description #=> "Collaborate with legal on developing doc storage solution"
# @return [String] the description
# @api public
#
class Milestone < Data.define(:id, :summary, :description); end
# rubocop:enable Style/DataInheritance
You have a few options:
Thanks for the response @lsegal!
I decided to go with your first example. I don't mind the extra line of Ruby code. In the second example, my IDE doesn't format correctly with the "comments nested in comments". So far as I can tell, the end result in the Ruby runtime environment is the same with or without the the extra class Milestone < Data ; end
.
One question: in your first example, I don't understand why you can't just replace the last line with:
# @!parse class Milestone < Data; end
I tried that and Milestone was reported as an undocumented object and in the generated documentation, the Milestone class was there but it only knew that it inherited from Data. None of the other class documentation was there.
Thanks for your help!
I have a similar situation. I have tried every variation of the above that I can, and none of them appear to remove the warning or actually render the correct documentation.
I kill the server, delete the .yardoc
directory, and restart using yard server --reload --bind 0.0.0.0
between each attempt so I don't think it's a caching issue.
I'm using Yard 0.9.34.
Here are the ways I've tried.
# @!parse class ModalComponent < LocoMotion::BaseComponent; end
class LocoMotion::Actions::ModalComponent < LocoMotion.configuration.component_class
# Lots of code here
end
class LocoMotion::Actions::ModalComponent < LocoMotion.configuration.component_class
# @!parse class ModalComponent < LocoMotion::BaseComponent; end
# Lots of code here
end
# @!parse ruby class ModalComponent < LocoMotion::BaseComponent; end
class LocoMotion::Actions::ModalComponent < LocoMotion.configuration.component_class
# Lots of code here
end
class LocoMotion::Actions::ModalComponent < LocoMotion.configuration.component_class
# @!parse ruby class ModalComponent < LocoMotion::BaseComponent; end
# Lots of code here
end
# @!parse
# # Create a <dialog>-based Modal component.
# # class LocoMotion::Actions::ModalComponent < LocoMotion::BaseComponent; end
class LocoMotion::Actions::ModalComponent < LocoMotion.configuration.component_class
# @!parse
# # Create a <dialog>-based Modal component.
# # class ModalComponent < LocoMotion::BaseComponent; end
class LocoMotion::Actions::ModalComponent < LocoMotion.configuration.component_class
Edit:
As a final note, I know that the @!parse
is running because if I mess up the code in the comment (like by removing the ; end
), it does generate a separate error about not being able to parse that documentation.
Ok, based on this comment, it appears that we cannot suppress the warning, and I suppose that is a separate issue we could open / discuss.
And after playing with it a bit more, I realized I was going about this the wrong way. Here is the code that did what I wanted:
# This is a Modal class!
# @!parse extend LocoMotion::BaseComponent
class LocoMotion::Actions::ModalComponent < LocoMotion.configuration.component_class
end
This forces Yard to say that the class inherits from BaseComponent
rather than Object
which is all I was really trying to get it to do 😄
Ok...I'm super confused.
The above code (# @!parse extend LocoMotion::BaseComponent
) stopped working and when I re-tried the following it DID work...
# This is a Modal component.
# @!parse class LocoMotion::Actions::ModalComponent < LocoMotion::BaseComponent; end
class LocoMotion::Actions::ModalComponent < LocoMotion.configuration.component_class
end
I guess something was somehow getting cached even though I was deleting the directory every time and re-running the server (maybe something client-side).
The only thing I can think of that changed between my first attempts and this latest / working one is that I declared the Actions
module and gave it some basic docs. Previously, I had left it undefined.
So maybe Yard was having trouble finding the base class when one of the modules in the inherited class' hierarchy wasn't defined?
The extend
keyword in Ruby does not define a superclass, it defines a mixin. Mixins are defined in parallel to class hierarcy. YARD would list the mixin as one of the ancestors, but it shouldn't be listing it as as the base class. Indeed following the steps in the comment you referenced is the correct way to do this, namely defining the class, not using extend. More importantly, extend applies to the metaclass, so it's not even really changing the ancestor chain of the class in the way you'd think (it's a bit meta).
IMO the solution here is the same as in the referenced issue, namely, you should just be removing the dynamism and defining the superclass directly:
class LocoMotion::Actions::ModalComponent < LocoMotion::BaseComponent
end
If this is indeed what the superclass is, I don't see the reason to use LocoMotion.configuration.component_class
at all. If the constant value can in fact be changed, then your documentation override isn't actually correct in suggesting that BaseComponent is the superclass.
I don't know much about this library, and I'm probably not in a place to recommend API changes here, but if the superclass is expected to change via user configuration, that is a very brittle and poorly formed API structure. If anything, this is the place where extend
(more appropriately, include) should be used in order to extend the class with extra functionality rather than dynamically changing the base class based on user configuration:
# make sure ModalComponent always contains _at least_ the features of BaseComponent
class LocoMotion::Actions::ModalComponent < LocoMotion::BaseComponent
include LocoMotion.configuration.component_class # allow extra component logic to be included
end
Of course, the cleanest way to allow for custom extensions would be to simply allow your downstream consumers to include
their own "components" rather than managing it for them in your base classes-- this is exactly why Rails generates ApplicationController type root classes rather than directly inheriting from ActionController::Base; this is what allows users to customize the inheritance chain without allowing them to completely pull the rug out from the inheritance chain. Either that, or use composition over inheritance for these extensions. The tl;dr here should be that if you're allowing users to customize the inherited members, that's fundamentally undocumentable to your library anyway.
@lsegal Yeah, the point of the configuration was allowing downstream users to change the base component class to their ApplicationComponent
in case they wanted to extend one of our components and make their own version.
I'm not actually sure we will need this, so maybe it's best to hold off on this (or similar) functionality for later.
Ultimately, though, I did get it working using the @!parse
mentioned above and I guess the caching made me think that the extend
was what did it.
Thanks for your response!
I am curious how I should document the following:
Such that YARD shows Milestone as a class with the attributes listed above. I can't seem to make it work.