Skip to content
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

NamespaceObject attr_writer statements #1016

Closed
MSP-Greg opened this issue Aug 14, 2016 · 3 comments
Closed

NamespaceObject attr_writer statements #1016

MSP-Greg opened this issue Aug 14, 2016 · 3 comments

Comments

@MSP-Greg
Copy link
Contributor

I'm working on seeing if I can get YARD / yard-t2 to recognize 'attributes' based on method properties, rather than a declaration, etc.

Came across the following, which seemed rather odd, as most of their 'readers' operate on a larger collection (children), the attributes hash, etc.

  class NamespaceObject < Base
    attr_writer :constants, :cvars, :mixins, :child, :meths
    attr_writer :class_attributes, :instance_attributes
    attr_writer :included_constants, :included_meths

See:

https://github.com/lsegal/yard/blob/master/lib/yard/code_objects/namespace_object.rb#L9

Obviously, I could search the file for instance variables, test it, etc, then generate a PR. But, there are limits to what testing will find, and there may be a purpose that I'm unaware of. Hence, the comment / issue.

@lsegal
Copy link
Owner

lsegal commented Aug 14, 2016

It looks like those were accidentally turned into writers in 66b4e93. Doesn't look like any were ever intended to be writable. All those lines could be removed (but it would technically be a breaking change).

@MSP-Greg
Copy link
Contributor Author

Maybe they should be left as is. After seeing you mention 'breaking change', I thought of the situation where someone could be subclassing NamespaceObject / ClassObject / ModuleObject, and expecting the writers to exist, along with replacing the reader method.

Maybe add a comment like 'These writers are not used by YARD and do not affect their respective reader functions. Provided for extensibility.'?

@lsegal
Copy link
Owner

lsegal commented Aug 15, 2016

Actually given the scenario here, I'm okay with removing the attributes. Although technically breaking, I would be really surprised if there was any code out there relying on these writers, since the code wouldn't be working correctly if it did.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants