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

Subclass documentation #121 #122

Merged
merged 1 commit into from
May 8, 2015
Merged

Subclass documentation #121 #122

merged 1 commit into from
May 8, 2015

Conversation

dan-corneanu
Copy link
Contributor

This is a proposed fix for issue
Subclasses keep the parent's .documentation? #121

Currently each Entity keeps track of its own exposures in a class object instance variable. A new entity class makes a copy of it's parent exposures then goes on and modifies its own exposures (add or remove exposures).

For the moment, I think we can fix the documentation issue by simply removing the call to superclass.documentation.

In the future we might have to replace the class object instance variable (which is a simple hash at the moment) with something more elaborate (like a chained filter or something) to handle the child - parent relationship better. Ex. what happens if we add some more exposures to the parent entity after we defined the child entity? I think that the child entity will not catch that extra exposure.

Please review this pull request and let me know what you think.

@dblock
Copy link
Member

dblock commented Apr 12, 2015

This looks good. Please update CHANGELOG and squash the commits.

@dan-corneanu
Copy link
Contributor Author

Thank you for reminding me.
Done

On Mon, Apr 13, 2015 at 1:29 AM, Daniel Doubrovkine (dB.) @dblockdotorg <
notifications@github.com> wrote:

This looks good. Please update CHANGELOG and squash the commits.


Reply to this email directly or view it on GitHub
#122 (comment).

@@ -3,6 +3,7 @@

* [#114](https://github.com/intridea/grape-entity/pull/114): Added 'only' option that selects which attributes should be returned - [@estevaoam](https://github.com/estevaoam).
* [#115](https://github.com/intridea/grape-entity/pull/115): Allowing 'root' to be inherited from parent to child entities - [@guidoprincess](https://github.com/guidoprincess).
* [#121](https://github.com/intridea/grape-entity/pull/122): Sublcassed Entity#documentation properly handles unexposed params. - [@dan-corneanu](https://github.com/dan-corneanu)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The period goes at the end, not after params :)

@dblock
Copy link
Member

dblock commented Apr 13, 2015

Can you please squash these and correct the super minor thing in CHANGELOG? Thanks.

@dan-corneanu
Copy link
Contributor Author

squashed

@dblock
Copy link
Member

dblock commented May 8, 2015

Merging, thanks.

dblock added a commit that referenced this pull request May 8, 2015
@dblock dblock merged commit ad72908 into ruby-grape:master May 8, 2015
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

Successfully merging this pull request may close these issues.

2 participants