colcon / colcon-mixin

Extension for colcon to read CLI mixins from files
http://colcon.readthedocs.io
Apache License 2.0
2 stars 7 forks source link

only unwrap default values in the mixin extension which have been wrapped by it #22

Closed dirk-thomas closed 4 years ago

dirk-thomas commented 4 years ago

Fixes #21 which is a regression of #19.

dirk-thomas commented 4 years ago

Is this something we have to worry about in other packages where unwrap_default_value is used?

I don't know of any other location where the function is being used. If you know any please point me to them so I can take a look to double check.

jacobperron commented 4 years ago

If you know any please point me to them so I can take a look to double check.

I'm not aware of any. I was thinking that it would be nice to have the original wrap and unwrap functions deal with this scenario so if it arises in other places we don't have to add similar workarounds. But, I don't have an alternative proposal, so this fix lgtm.