Skip to content
This repository has been archived by the owner on Mar 19, 2019. It is now read-only.

Parameters cannot be inherited #646

Closed
Lawouach opened this issue Jul 7, 2017 · 5 comments
Closed

Parameters cannot be inherited #646

Lawouach opened this issue Jul 7, 2017 · 5 comments

Comments

@Lawouach
Copy link
Contributor

Lawouach commented Jul 7, 2017

Assuming you have a rug definining parameters through the decorator. When you inherit from that rug and add new parameters, the ones from the base class do not appear in the subclass rug's definition.

Note that if your subclass doesn't add parameters at all, then the base class ones will show up in that definition.

I think we should have a consistant behavior that parameters can be inherited.

Likely because this function doesn't walk up the chain: https://github.com/atomist/rug/blob/master/src/main/typescript/rug/operations/Decorators.ts#L19

@kipz
Copy link
Contributor

kipz commented Jul 10, 2017

@Lawouach what behaviour do you want here? In the general case get_metadata is behaving well I think. In this specific case, do you want the object and its prototype's parameters to be merged? What if there are conflicts?

@Lawouach
Copy link
Contributor Author

I think my gut feeling says merging and overriding. Not sure what you mean by conflicts here.

If not, we should find a way not to surprise users as I were. It is a tricky one to track because, though I can inherit from handlers, their parameters aren't passed down as I was expecting. It's only when I saw a log without the base parameters that I realised of that behavior.

That being said, this article led me to that all the way to the init() pattern that we used to workaround the limit I describe here.

Maybe, we could promote such a solution more clearly as well?

@kipz
Copy link
Contributor

kipz commented Jul 10, 2017

By conflicts, I mean that a Parameter with the same name exists in more than one place. Overriding would be a way to resolve these conflicts. It'd be pretty easy to fix this in the @parameter decorator, by just grabbing the parameter list from the prototype, and merging & overriding with the inheriting object.

@Lawouach
Copy link
Contributor Author

Maybe, it's just me but the least surprising would indeed be overriding. Maybe this is the wrong assumption though.

@kipz kipz self-assigned this Jul 12, 2017
ddgenome added a commit that referenced this issue Jul 12, 2017
Inherit/merge parameters from prototype #646
@kipz
Copy link
Contributor

kipz commented Jul 12, 2017

Fixed in master.

@kipz kipz closed this as completed Jul 12, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants