orocos-toolchain / orogen

Code generator for components and type handling in Rock - the Robot Construction Kit - and the Orocos Toolchain
http://rock-robotics.org
Other
4 stars 35 forks source link

Cleanup of plugin-hooks #60

Closed goldhoorn closed 9 years ago

goldhoorn commented 9 years ago

This PR is for @jmachowinski in detail #43. The plugin is not (yet) ported, but i would like to ask @doudou for his opinion.

goldhoorn commented 9 years ago

@marvin2k you are one of the internal reviewers too (cc)

goldhoorn commented 9 years ago

to clarify, main "owner" is @doudou who has to agree

doudou commented 9 years ago

Overall, the functionality seem useful and well-integrated.

goldhoorn commented 9 years ago

@doudou i like the "hook" naming more thatn "generation", hook make kind of clear that it's a kind of callback from the plugin and it is (for me) from a semantic point clearer. "early" is kind of "somewhere early but unknown when" for me. "pre" sayes is really "before".

However if you want to keep the old naming i remove the renaming part from this commit and leave only the extension. I updated the PR according the other remarks...

goldhoorn commented 9 years ago

btw. you requested each_ i would like to prevent to force the user to write "inherit_enumerable" within his plugins it only additional magic which makes the plugin system unnecessary more complex and harder to understand for new-developers.

doudou commented 9 years ago

(1) we are talking about people writing orogen plugins. That's not your standard developer. (2) a each_ method is a METHOD. It is written all the time without using inerhited_enumerable, which in any case should only be used when inheritance is a concern (i.e. if one would like its plugins to "inherit" the auto-folders that its parent class defines). Definitely not a primary concern here, I would DEFINITELY not use inheritedenumerable, but simply define the each method. (3) a each_ method would be a great fit for a base plugin class (which would be a good thing to have, if only to define a "template" for plugins and document their development/integration).

doudou commented 9 years ago

And I do agree with the new naming for hooks

goldhoorn commented 9 years ago

updated

doudou commented 9 years ago

Still not using the #each_ method.

goldhoorn commented 9 years ago

Maybe i should push my stuff to github ;) updated