-
Notifications
You must be signed in to change notification settings - Fork 27
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
Document SignatureBuildOrder
cop
#186
Conversation
Not having documentation means that `rubocop:todo` comments attached to the class end up erroneously treated as documentation, and the simplest way to avoid that is to document the doc, which we should be doing anyway.
3c34356
to
e570049
Compare
# - type_parameters | ||
# - params | ||
# - returns, or void | ||
# - soft, checked, or on_failure |
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.
When checking the docs for these (so I could know if they were mutually exclusive), I couldn't find the soft
method in https://github.com/sorbet/sorbet/blob/4ef2b632ae1433ed1a544fd568f3481a4fc8f2df/rbi/sorbet/builder.rbi.
I did notice that there are a couple methods which we don't include in our list, which we should probably add:
final
implementation
bind
It might even be worth having a test that checks that the constant contains every method defined on T::Private::Methods::DeclBuilder
... 🤔
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.
soft
has been renamed into on_failure
a while ago but can theoretically still be found in codebases using an old Sorbet.
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 did notice that there are a couple methods which we don't include in our list, which we should probably add:
- final
- implementation
- bind
Let's add them 👍
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.
Sounds good. I've opened #189 to do that, to unblock this PR (and the subsequent ones branched off of it, and keep it about the documentation, especially since I need to figure out what implementation
is and where it belongs 😅).
dbbdb2c
to
e570049
Compare
Not having documentation means that
rubocop:todo
comments attached to the class end up erroneously treated as documentation, and the simplest way to avoid that is to document the doc, which we should be doing anyway.See #183 for why this is relevant.