-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Separation of constructor methods in docs #4216
Conversation
Mmh in general I like this, what I would even more like however is if it also would include any factory methods, so I come to wonder if this wouldn't decrease the visibility of these, since you will no longer scan the class methods in order to discover the ways you can get an instance of the class. |
@jhass I'd love to include factory methods as well, alas I couldn't find any decent way to filter them out of regular class method list (except by checking return type which is not that reliable since it might be unset). |
Maybe we should add something like
|
👍 for |
NB: If a comment is applied to mark as factory, then the subtraction of methods needs to be done on class and instance methods. class Nat
# :factory:
def self.zero
end
# :factory:
def succ
end
end |
Do we really want to start introducing some documentation-specific annotations? Instead, what about using the specified return type of class methods (which the type inference already uses)? Maybe it could use the inferred type, too. If a class method of class X returns a X, then it's a factory. |
@ysbaddaden Would be perfect, yet return type annotations are not always available IIUC. |
Then specifying return type of factories would have double bonus:
Eventually, could the doc generator infer types? just like the compiler does (first pass)? that seems doable, to me. |
@ysbaddaden I've pushed another commit adding factory methods to the list. |
Not necessarily. The type inference act upon invocation. The only invocations could be the one in the specs. I'm ok with using an explicit return type as a flag to identify factory methods. |
Anything more to do here before merging? |
@Sija I think this needs a bit more discussion, though initially I really like the idea :-) |
@asterite Sure thing, I was a bit too hasty with "before merging" part ;) |
dadae8f
to
5626e2f
Compare
Now, when the check is more permissive, singleton methods like |
@asterite any decisions yet? |
We can separate |
That's true, there are cases where this distinction can't be done automagically. To combine best of both worlds I'd go for new section titled "Factory methods" in which |
@asterite ping? r we ready for some action? :) |
@asterite ping :) |
@Sija I like it! Thank you! 🎉 |
Simple implementation of idea from issue #4071.