[xwiki-devs] Activity Stream no more properly subclassable in 3.5?
hello devs, having moved Curriki to the 3.5 core, we noticed our activity stream was having empty getDisplayBody... looking more closely revealed that the objects were all of type ActivityEvent while a family of sub-classes was available in the CurrikiActivityStream: the method ActivityStreamPluginApi.wrapEvents was made private hence its subclass was ignored! This appears to have been made at https://github.com/xwiki/xwiki-platform/commit/1837196f0f6434603c4c24a13b7c1... as part of a "code cleanup". Jean-Vincent, or someone else, could you explain the rationale behind it? I am sure this was checked for "others usages" but Curriki was not considered as part of that. Having somewhat fixed this on my side by bringing more methods to the subclass (for the display of older events), I come to realize that newer events are also not of the right type so I'll have to hunt more for "compromised subclassing"... This is rather an API breakage to my taste. Has there been a policy about this? Could we set-up one? I wonder if such a search engine exists that would have indicated that such a breakage would have been avoided. paul
On Wed, Apr 11, 2012 at 8:52 PM, Paul Libbrecht <[email protected]> wrote:
hello devs,
having moved Curriki to the 3.5 core, we noticed our activity stream was having empty getDisplayBody... looking more closely revealed that the objects were all of type ActivityEvent while a family of sub-classes was available in the CurrikiActivityStream: the method ActivityStreamPluginApi.wrapEvents was made private hence its subclass was ignored!
This appears to have been made at https://github.com/xwiki/xwiki-platform/commit/1837196f0f6434603c4c24a13b7c1... as part of a "code cleanup". Jean-Vincent, or someone else, could you explain the rationale behind it? I am sure this was checked for "others usages" but Curriki was not considered as part of that.
I had forgotten I did this refactoring/cleanup. It seems I wrongly considered those APIs as internal, this is indeed a regression. I'll fix it ASAP.
Having somewhat fixed this on my side by bringing more methods to the subclass (for the display of older events), I come to realize that newer events are also not of the right type so I'll have to hunt more for "compromised subclassing"...
This is rather an API breakage to my taste. Has there been a policy about this? Could we set-up one? I wonder if such a search engine exists that would have indicated that such a breakage would have been avoided.
I think clirr is doing this job now. -- Jean-Vincent Drean, XWiki.
Le 13 avr. 2012 à 17:27, Jean-Vincent Drean a écrit :
This appears to have been made at https://github.com/xwiki/xwiki-platform/commit/1837196f0f6434603c4c24a13b7c1... as part of a "code cleanup". Jean-Vincent, or someone else, could you explain the rationale behind it? I am sure this was checked for "others usages" but Curriki was not considered as part of that.
I had forgotten I did this refactoring/cleanup.
This is long ago! We're getting awake!
It seems I wrongly considered those APIs as internal, this is indeed a regression. I'll fix it ASAP.
thanks, it is well fixed in 3.5.x and 4.0 branches.
Having somewhat fixed this on my side by bringing more methods to the subclass (for the display of older events), I come to realize that newer events are also not of the right type so I'll have to hunt more for "compromised subclassing"...
This is rather an API breakage to my taste. Has there been a policy about this? Could we set-up one? I wonder if such a search engine exists that would have indicated that such a breakage would have been avoided. I think clirr is doing this job now.
Could we configure it to consider projects such as Curriki? paul
On Apr 13, 2012, at 5:46 PM, Paul Libbrecht wrote:
Le 13 avr. 2012 à 17:27, Jean-Vincent Drean a écrit :
This appears to have been made at https://github.com/xwiki/xwiki-platform/commit/1837196f0f6434603c4c24a13b7c1... as part of a "code cleanup". Jean-Vincent, or someone else, could you explain the rationale behind it? I am sure this was checked for "others usages" but Curriki was not considered as part of that.
I had forgotten I did this refactoring/cleanup.
This is long ago! We're getting awake!
It seems I wrongly considered those APIs as internal, this is indeed a regression. I'll fix it ASAP.
thanks, it is well fixed in 3.5.x and 4.0 branches.
Having somewhat fixed this on my side by bringing more methods to the subclass (for the display of older events), I come to realize that newer events are also not of the right type so I'll have to hunt more for "compromised subclassing"...
This is rather an API breakage to my taste. Has there been a policy about this? Could we set-up one? I wonder if such a search engine exists that would have indicated that such a breakage would have been avoided. I think clirr is doing this job now.
Could we configure it to consider projects such as Curriki?
It has nothing to do with Curriki. See http://dev.xwiki.org/xwiki/bin/view/Community/DevelopmentPractices#HBackward... Thanks -Vincent
Le 13 avr. 2012 à 17:53, Vincent Massol a écrit :
Le 13 avr. 2012 à 17:27, Jean-Vincent Drean a écrit :
This appears to have been made at https://github.com/xwiki/xwiki-platform/commit/1837196f0f6434603c4c24a13b7c1... as part of a "code cleanup". Jean-Vincent, or someone else, could you explain the rationale behind it? I am sure this was checked for "others usages" but Curriki was not considered as part of that. [...] I wonder if such a search engine exists that would have indicated that such a breakage would have been avoided. I think clirr is doing this job now. Could we configure it to consider projects such as Curriki? It has nothing to do with Curriki.
I was expecting a tool that evaluates usages so that going private would have been warned against.
See http://dev.xwiki.org/xwiki/bin/view/Community/DevelopmentPractices#HBackward...
Good, so there's a software to watchdog backwards compatibility. So, was clirr not yet available when this commit was made? ("3 years ago"). paul
Hi Paul, On Apr 13, 2012, at 8:26 PM, Paul Libbrecht wrote:
Le 13 avr. 2012 à 17:53, Vincent Massol a écrit :
Le 13 avr. 2012 à 17:27, Jean-Vincent Drean a écrit :
This appears to have been made at https://github.com/xwiki/xwiki-platform/commit/1837196f0f6434603c4c24a13b7c1... as part of a "code cleanup". Jean-Vincent, or someone else, could you explain the rationale behind it? I am sure this was checked for "others usages" but Curriki was not considered as part of that. [...] I wonder if such a search engine exists that would have indicated that such a breakage would have been avoided. I think clirr is doing this job now. Could we configure it to consider projects such as Curriki? It has nothing to do with Curriki.
I was expecting a tool that evaluates usages so that going private would have been warned against.
See http://dev.xwiki.org/xwiki/bin/view/Community/DevelopmentPractices#HBackward...
Good, so there's a software to watchdog backwards compatibility.
So, was clirr not yet available when this commit was made? ("3 years ago").
It was maybe available. But as I mentioned, when we voluntarily break an API we put that in the release notes. So we may have decided voluntarily to break this API. And honestly that wasn't a bad choice. In 3 years you're the only person affected which means it was a pretty good choice to make with very limited breakage ;) JV has decided to put it back hastily because probably he didn't have the time to spend some time with you to analyze your need and see how to best answer it. Now it's not a bad choice he just did since we're going to completely rewrite this plugin as components one day (I hope that day isn't too far in the future ;)) and the whole AS plugin will be moved to our legacy modules when it happens. At that point you'll be able to either continue using the old legacy stuff or move to the new one and you should definitely raise your needs/use cases before we finish the rewrite… ;) Thanks -Vincent
On Fri, Apr 13, 2012 at 5:27 PM, Jean-Vincent Drean <[email protected]> wrote:
On Wed, Apr 11, 2012 at 8:52 PM, Paul Libbrecht <[email protected]> wrote:
hello devs,
having moved Curriki to the 3.5 core, we noticed our activity stream was having empty getDisplayBody... looking more closely revealed that the objects were all of type ActivityEvent while a family of sub-classes was available in the CurrikiActivityStream: the method ActivityStreamPluginApi.wrapEvents was made private hence its subclass was ignored!
This appears to have been made at https://github.com/xwiki/xwiki-platform/commit/1837196f0f6434603c4c24a13b7c1... as part of a "code cleanup". Jean-Vincent, or someone else, could you explain the rationale behind it? I am sure this was checked for "others usages" but Curriki was not considered as part of that.
I had forgotten I did this refactoring/cleanup. It seems I wrongly considered those APIs as internal, this is indeed a regression. I'll fix it ASAP.
Done: http://jira.xwiki.org/browse/XWIKI-7731
Having somewhat fixed this on my side by bringing more methods to the subclass (for the display of older events), I come to realize that newer events are also not of the right type so I'll have to hunt more for "compromised subclassing"...
This is rather an API breakage to my taste. Has there been a policy about this? Could we set-up one? I wonder if such a search engine exists that would have indicated that such a breakage would have been avoided.
I think clirr is doing this job now.
-- Jean-Vincent Drean, XWiki.
-- Jean-Vincent Drean, XWiki.
participants (3)
-
Jean-Vincent Drean -
Paul Libbrecht -
Vincent Massol