Community
Participate
Working Groups
Following the discussion in [1] the default error handling for dealing with missing unbind methods should be changed to ignore, as the unbind method is not mandatory. [1] http://stackoverflow.com/questions/36932825/osgi-modify-event-method-not-called
@Peter What do you think about this? Did you have a specific reason for setting the default to Error?
It's defaulted this way in order to preserve backward-compatibility behavior with releases previous to this fix. Keep in mind this plugin has been used in production for years before it came to PDE. Since it's new to PDE we can default it to false, but in practice I find that this is by far the most common developer mistake (to mis-name the implicit unbind method accidentally).
(In reply to Peter Nehrer from comment #2) > It's defaulted this way in order to preserve backward-compatibility behavior > with releases previous to this fix. By "this fix" I mean https://github.com/ecliptical/ds-annotations/issues/15.
I understand ... a discussion about defaults is always not that easy. While on the one hand I'm completely with you that this could be one of the most common mistakes that is hard to find, it leads on the other hand to misunderstandings of the spec. For example, because of the error I thought that it is necessary to specify an unbind method to follow the specification. Now I know that it is not. So the error is somehow misleading. Not sure, what do you think? Maybe make it a warning and modify the message a little bit?
I'm fine with it defaulting to false in PDE. Warning would work as well.
I have to correct myself. It seems I misinterpreted the comment in the linked discussion. Although it is not necessary it seems to be a common and best practice to specify an unbind method. At least for dynamic references. Even bndtools is showing an error in this case. I therefore tend to keep the error as default. But maybe it makes sense to restrict the warning only to dynamic references. What do you think? There was a discussion here https://groups.google.com/forum/m/#!topic/bndtools-users/tFJES5f3R0I
So are you suggesting that we don't keep the preference and just flag missing unbind for dynamic references as error, OR, that we keep the preference but apply it to dynamic references only?
(In reply to Peter Nehrer from comment #7) > So are you suggesting that we don't keep the preference and just flag > missing unbind for dynamic references as error, OR, that we keep the > preference but apply it to dynamic references only? I would keep the preference but only apply it to dynamic references. That would fit the bnd recommendations by still leaving the option to change the behavior. For static references it seems to be really not necessary to provide an unbind method because of re-activation of a component configuration.
New Gerrit change created: https://git.eclipse.org/r/79021
Gerrit change https://git.eclipse.org/r/79021 was merged to [master]. Commit: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=c2a2d69f829a85f9d24d19d135e4b2e41a7cf60d
Peter, if this is fixed via previous commit, can you please resolve this defect. Thanks !
Dirk, can you please verify this fix. Thanks!
Tested with I20160906-0800 and works as expected
