[xwiki-devs] [Discussion] Should we re-think the way we determine what is user-public code ?
Hi devs, Following a thread on github, here's a mail to start a discussion about the way we determine what is API or not. Our current rule is : 1) "Non user-public code must be located in an internal package just after the module name." c.f. [1] (implied that what is not in internal is public) 2) What is public has to go through the deprecation strategy described at [2]). I think the rule is good but there is a problem in its enforcement right now, mainly because : * There is some legacy code where 1) does not make much sense because it "arrived too late at the party" (for example in oldcore) * There is some "new code" where some classes/interfaces hasn't been made internal when they likely should have been. For example I've been playing around with the WYSIWYG recently and in the client module, there are *only* user-public classes [3]. I'm sure we can find a lot of examples of such practice and I'm OK to work and list some if needed. The problem I see coming is that the cost of refactoring will increase for a lot of modules/classes/etc., while at the same time those modules should not have been API to begin with, and are probably not even being used as such by anybody. The risk is our progress being bogged down for no good reason. So what can we do to enforce a solid backward-compatibility policy for API code, while keeping flexibility in XWiki internals ? We could : A) Not do anything :) Maybe it's just me that sees this as a potential problem. B) Do the work to move everything that ought to be in an internal package to an internal package. But I don't believe we're reading to make that effort, so that's not going to happen IMHO C) Do the work to move what ought to be in an internal package "on the fly", when refactoring code. That would be on a case-by-case case, probably requiring a mail to announce it ; or more coercive, a VOTE. C) We change the rule. We could decide that instead of having everything be an API and enforce the "internal" status in a special package, we take it the other way around and Day everything is internal, and APIs needs to be in a special package (or be annotated with a special annotation). This could also be the opportunity to introduce another rule that says that such APIs should be referenced in their own module documentation on extensions.xwiki.org. What do you think ? Jerome. [1] http://dev.xwiki.org/xwiki/bin/view/Community/JavaCodeStyle#HPackagenames [2] http://dev.xwiki.org/xwiki/bin/view/Community/DevelopmentPractices#HBackward...) [3] https://github.com/xwiki/xwiki-platform/tree/master/xwiki-platform-core/xwik... -- Jérôme Velociter Winesquare http://www.winesquare.net/
On Mon, May 21, 2012 at 7:12 PM, Jerome Velociter <[email protected]> wrote:
Hi devs,
Following a thread on github, here's a mail to start a discussion about the way we determine what is API or not.
Our current rule is : 1) "Non user-public code must be located in an internal package just after the module name." c.f. [1] (implied that what is not in internal is public) 2) What is public has to go through the deprecation strategy described at [2]).
I think the rule is good but there is a problem in its enforcement right now, mainly because : * There is some legacy code where 1) does not make much sense because it "arrived too late at the party" (for example in oldcore) * There is some "new code" where some classes/interfaces hasn't been made internal when they likely should have been. For example I've been playing around with the WYSIWYG recently and in the client module, there are *only* user-public classes [3]. I'm sure we can find a lot of examples of such practice and I'm OK to work and list some if needed.
The problem I see coming is that the cost of refactoring will increase for a lot of modules/classes/etc., while at the same time those modules should not have been API to begin with, and are probably not even being used as such by anybody. The risk is our progress being bogged down for no good reason. So what can we do to enforce a solid backward-compatibility policy for API code, while keeping flexibility in XWiki internals ?
We could : A) Not do anything :) Maybe it's just me that sees this as a potential problem. B) Do the work to move everything that ought to be in an internal package to an internal package. But I don't believe we're reading to make that effort, so that's not going to happen IMHO C) Do the work to move what ought to be in an internal package "on the fly", when refactoring code. That would be on a case-by-case case, probably requiring a mail to announce it ; or more coercive, a VOTE. C) We change the rule. We could decide that instead of having everything be an API and enforce the "internal" status in a special package, we take it the other way around and Day everything is internal, and APIs needs to be in a special package (or be annotated with a special annotation). This could also be the opportunity to introduce another rule that says that such APIs should be referenced in their own module documentation on extensions.xwiki.org.
What do you think ?
Big -1 to force putting public APIs in a specific useless package. The main issue is mostly with oldcore which is supposed to be refactored at some point peace by peace anyway so that's not a big issue IMO. It's just another argument to get rid of it ASAP. For "new" modules, if some code is is public while it should not then it should be fixed ASAP and indicated in the release note (since it's a breakage even if it should never have existed in the first place) or made @Deprecated if it's too old and jugged probably already used to be removed like that, in both cases it would require a VOTE IMO. Note sure if that's B or C. The status of new code is not as bad as you think IMO and I don't see the point in changing a good rule because it's not applicable to old code that's supposed to be replaced anyway at some point. Whatever the new rule you will never be able to apply it to oldcore anyway... For example C does not work since we have a lot of things we do want public for java code which are not in the "api" package and we are not going to deprecated and move them there since it does not worth the effort.
Jerome.
[1] http://dev.xwiki.org/xwiki/bin/view/Community/JavaCodeStyle#HPackagenames [2] http://dev.xwiki.org/xwiki/bin/view/Community/DevelopmentPractices#HBackward...) [3] https://github.com/xwiki/xwiki-platform/tree/master/xwiki-platform-core/xwik...
-- Jérôme Velociter Winesquare http://www.winesquare.net/ _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne
On Tue, May 22, 2012 at 9:08 AM, Thomas Mortagne <[email protected]> wrote:
On Mon, May 21, 2012 at 7:12 PM, Jerome Velociter <[email protected]> wrote:
Hi devs,
Following a thread on github, here's a mail to start a discussion about the way we determine what is API or not.
Our current rule is : 1) "Non user-public code must be located in an internal package just after the module name." c.f. [1] (implied that what is not in internal is public) 2) What is public has to go through the deprecation strategy described at [2]).
I think the rule is good but there is a problem in its enforcement right now, mainly because : * There is some legacy code where 1) does not make much sense because it "arrived too late at the party" (for example in oldcore) * There is some "new code" where some classes/interfaces hasn't been made internal when they likely should have been. For example I've been playing around with the WYSIWYG recently and in the client module, there are *only* user-public classes [3]. I'm sure we can find a lot of examples of such practice and I'm OK to work and list some if needed.
The problem I see coming is that the cost of refactoring will increase for a lot of modules/classes/etc., while at the same time those modules should not have been API to begin with, and are probably not even being used as such by anybody. The risk is our progress being bogged down for no good reason. So what can we do to enforce a solid backward-compatibility policy for API code, while keeping flexibility in XWiki internals ?
We could : A) Not do anything :) Maybe it's just me that sees this as a potential problem. B) Do the work to move everything that ought to be in an internal package to an internal package. But I don't believe we're reading to make that effort, so that's not going to happen IMHO C) Do the work to move what ought to be in an internal package "on the fly", when refactoring code. That would be on a case-by-case case, probably requiring a mail to announce it ; or more coercive, a VOTE. C) We change the rule. We could decide that instead of having everything be an API and enforce the "internal" status in a special package, we take it the other way around and Day everything is internal, and APIs needs to be in a special package (or be annotated with a special annotation). This could also be the opportunity to introduce another rule that says that such APIs should be referenced in their own module documentation on extensions.xwiki.org.
What do you think ?
Big -1 to force putting public APIs in a specific useless package.
The main issue is mostly with oldcore which is supposed to be refactored at some point peace by peace anyway so that's not a big issue IMO. It's just another argument to get rid of it ASAP.
For "new" modules, if some code is is public while it should not then it should be fixed ASAP and indicated in the release note (since it's a breakage even if it should never have existed in the first place) or made @Deprecated if it's too old and jugged probably already used to be removed like that, in both cases it would require a VOTE IMO. Note sure if that's B or C.
The status of new code is not as bad as you think IMO
Besides the WYSIWYG, there is the REST module that also has (wrongly) only public code. For other modules it looks better, though I think there are quite some interfaces that are offered as public for the sheer fact they are interfaces ; which is a mistake IMO. Also, there is the case of the DocumentAccessBridge, which by essence is meant to grow, thus be broken (but maybe it is "complete" now, though if you think about it, our present rule would not have permit us to elaborate it in the time-frame we did - which is a couple of years) Jerome
and I don't see the point in changing a good rule because it's not applicable to old code that's supposed to be replaced anyway at some point. Whatever the new rule you will never be able to apply it to oldcore anyway... For example C does not work since we have a lot of things we do want public for java code which are not in the "api" package and we are not going to deprecated and move them there since it does not worth the effort.
Jerome.
[1] http://dev.xwiki.org/xwiki/bin/view/Community/JavaCodeStyle#HPackagenames [2] http://dev.xwiki.org/xwiki/bin/view/Community/DevelopmentPractices#HBackward...) [3] https://github.com/xwiki/xwiki-platform/tree/master/xwiki-platform-core/xwik...
-- Jérôme Velociter Winesquare http://www.winesquare.net/ _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Jérôme Velociter Winesquare http://www.winesquare.net/
Hi Jerome, On May 21, 2012, at 7:12 PM, Jerome Velociter wrote:
Hi devs,
Following a thread on github, here's a mail to start a discussion about the way we determine what is API or not.
Our current rule is : 1) "Non user-public code must be located in an internal package just after the module name." c.f. [1] (implied that what is not in internal is public) 2) What is public has to go through the deprecation strategy described at [2]).
I think the rule is good but there is a problem in its enforcement right now, mainly because : * There is some legacy code where 1) does not make much sense because it "arrived too late at the party" (for example in oldcore) * There is some "new code" where some classes/interfaces hasn't been made internal when they likely should have been. For example I've been playing around with the WYSIWYG recently and in the client module, there are *only* user-public classes [3]. I'm sure we can find a lot of examples of such practice and I'm OK to work and list some if needed.
The problem I see coming is that the cost of refactoring will increase for a lot of modules/classes/etc., while at the same time those modules should not have been API to begin with, and are probably not even being used as such by anybody. The risk is our progress being bogged down for no good reason. So what can we do to enforce a solid backward-compatibility policy for API code, while keeping flexibility in XWiki internals ?
I agree with you that all developers needs to be more careful about where they put code and they should favor the internal package (i.e code defensively) and open up APIs only when asked by someone or needed by some other modules.
We could : A) Not do anything :) Maybe it's just me that sees this as a potential problem. B) Do the work to move everything that ought to be in an internal package to an internal package. But I don't believe we're reading to make that effort, so that's not going to happen IMHO C) Do the work to move what ought to be in an internal package "on the fly", when refactoring code. That would be on a case-by-case case, probably requiring a mail to announce it ; or more coercive, a VOTE. C) We change the rule. We could decide that instead of having everything be an API and enforce the "internal" status in a special package, we take it the other way around and Day everything is internal, and APIs needs to be in a special package (or be annotated with a special annotation). This could also be the opportunity to introduce another rule that says that such APIs should be referenced in their own module documentation on extensions.xwiki.org.
What do you think ?
C is too much work. I prefer to keep the current rule, i.e. internal for non user-public work. IMO we should review modules one by one and list potential classes that should be internal instead. Now for old modules (oldcore + plugins) since we need to rewrite them IMO we just need to continue our rewriting process and when we rewrite them take that occasion to move the max stuff in the internal package. I don't think we should touch existing oldcore/plugins code though to not break backward compat. since we need to move that code to new modules anyway and at that time we'll move old code to legacy modules. So, unlike you, I don't think we have a major generic problem. I think we just need to be careful and make sure that committers review code committed by others and when doing this review take care to check the package. This means raising the awareness of backward-compatibility in general which is what I'm trying to achieve with the new Deprecation/Legacy policy. Note to self: need to conclude on young APIs. Thanks -Vincent
Jerome.
[1] http://dev.xwiki.org/xwiki/bin/view/Community/JavaCodeStyle#HPackagenames [2] http://dev.xwiki.org/xwiki/bin/view/Community/DevelopmentPractices#HBackward...) [3] https://github.com/xwiki/xwiki-platform/tree/master/xwiki-platform-core/xwik...
-- Jérôme Velociter
On Tue, May 22, 2012 at 11:50 AM, Vincent Massol <[email protected]> wrote:
Hi Jerome,
On May 21, 2012, at 7:12 PM, Jerome Velociter wrote:
Hi devs,
Following a thread on github, here's a mail to start a discussion about the way we determine what is API or not.
Our current rule is : 1) "Non user-public code must be located in an internal package just after the module name." c.f. [1] (implied that what is not in internal is public) 2) What is public has to go through the deprecation strategy described at [2]).
I think the rule is good but there is a problem in its enforcement right now, mainly because : * There is some legacy code where 1) does not make much sense because it "arrived too late at the party" (for example in oldcore) * There is some "new code" where some classes/interfaces hasn't been made internal when they likely should have been. For example I've been playing around with the WYSIWYG recently and in the client module, there are *only* user-public classes [3]. I'm sure we can find a lot of examples of such practice and I'm OK to work and list some if needed.
The problem I see coming is that the cost of refactoring will increase for a lot of modules/classes/etc., while at the same time those modules should not have been API to begin with, and are probably not even being used as such by anybody. The risk is our progress being bogged down for no good reason. So what can we do to enforce a solid backward-compatibility policy for API code, while keeping flexibility in XWiki internals ?
I agree with you that all developers needs to be more careful about where they put code and they should favor the internal package (i.e code defensively) and open up APIs only when asked by someone or needed by some other modules.
We could : A) Not do anything :) Maybe it's just me that sees this as a potential problem. B) Do the work to move everything that ought to be in an internal package to an internal package. But I don't believe we're reading to make that effort, so that's not going to happen IMHO C) Do the work to move what ought to be in an internal package "on the fly", when refactoring code. That would be on a case-by-case case, probably requiring a mail to announce it ; or more coercive, a VOTE. C) We change the rule. We could decide that instead of having everything be an API and enforce the "internal" status in a special package, we take it the other way around and Day everything is internal, and APIs needs to be in a special package (or be annotated with a special annotation). This could also be the opportunity to introduce another rule that says that such APIs should be referenced in their own module documentation on extensions.xwiki.org.
What do you think ?
C is too much work. I prefer to keep the current rule, i.e. internal for non user-public work.
Sorry, last item should have been D) of course.
IMO we should review modules one by one and list potential classes that should be internal instead.
Yes, that's what I've started to take a look at to see if it was just my imagination :) I think doing it all at once is too much work, that's why I proposed in item C) to do it when actually doing something that impact such code. For example, let say I refactor something in the REST module, and realize there are some classes that shouldn't have been made public to begin with, I send a mail (possibly a VOTE?) to explain the deal and if we agree, the incriminating code is moved into internal when doing the refactoring. Of course it also means we document the breakage in the release notes
Now for old modules (oldcore + plugins) since we need to rewrite them IMO we just need to continue our rewriting process and when we rewrite them take that occasion to move the max stuff in the internal package.
I don't think we should touch existing oldcore/plugins code though to not break backward compat. since we need to move that code to new modules anyway and at that time we'll move old code to legacy modules.
So, unlike you, I don't think we have a major generic problem. I think we just need to be careful and make sure that committers review code committed by others and when doing this review take care to check the package. This means raising the awareness of backward-compatibility in general which is what I'm trying to achieve with the new Deprecation/Legacy policy. Note to self: need to conclude on young APIs.
Thanks -Vincent
Jerome.
[1] http://dev.xwiki.org/xwiki/bin/view/Community/JavaCodeStyle#HPackagenames [2] http://dev.xwiki.org/xwiki/bin/view/Community/DevelopmentPractices#HBackward...) [3] https://github.com/xwiki/xwiki-platform/tree/master/xwiki-platform-core/xwik...
-- Jérôme Velociter
devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Jérôme Velociter Winesquare http://www.winesquare.net/
On May 22, 2012, at 12:06 PM, Jerome Velociter wrote:
On Tue, May 22, 2012 at 11:50 AM, Vincent Massol <[email protected]> wrote:
Hi Jerome,
On May 21, 2012, at 7:12 PM, Jerome Velociter wrote:
Hi devs,
Following a thread on github, here's a mail to start a discussion about the way we determine what is API or not.
Our current rule is : 1) "Non user-public code must be located in an internal package just after the module name." c.f. [1] (implied that what is not in internal is public) 2) What is public has to go through the deprecation strategy described at [2]).
I think the rule is good but there is a problem in its enforcement right now, mainly because : * There is some legacy code where 1) does not make much sense because it "arrived too late at the party" (for example in oldcore) * There is some "new code" where some classes/interfaces hasn't been made internal when they likely should have been. For example I've been playing around with the WYSIWYG recently and in the client module, there are *only* user-public classes [3]. I'm sure we can find a lot of examples of such practice and I'm OK to work and list some if needed.
The problem I see coming is that the cost of refactoring will increase for a lot of modules/classes/etc., while at the same time those modules should not have been API to begin with, and are probably not even being used as such by anybody. The risk is our progress being bogged down for no good reason. So what can we do to enforce a solid backward-compatibility policy for API code, while keeping flexibility in XWiki internals ?
I agree with you that all developers needs to be more careful about where they put code and they should favor the internal package (i.e code defensively) and open up APIs only when asked by someone or needed by some other modules.
We could : A) Not do anything :) Maybe it's just me that sees this as a potential problem. B) Do the work to move everything that ought to be in an internal package to an internal package. But I don't believe we're reading to make that effort, so that's not going to happen IMHO C) Do the work to move what ought to be in an internal package "on the fly", when refactoring code. That would be on a case-by-case case, probably requiring a mail to announce it ; or more coercive, a VOTE. C) We change the rule. We could decide that instead of having everything be an API and enforce the "internal" status in a special package, we take it the other way around and Day everything is internal, and APIs needs to be in a special package (or be annotated with a special annotation). This could also be the opportunity to introduce another rule that says that such APIs should be referenced in their own module documentation on extensions.xwiki.org.
What do you think ?
C is too much work. I prefer to keep the current rule, i.e. internal for non user-public work.
Sorry, last item should have been D) of course.
IMO we should review modules one by one and list potential classes that should be internal instead.
Yes, that's what I've started to take a look at to see if it was just my imagination :)
:)
I think doing it all at once is too much work, that's why I proposed in item C) to do it when actually doing something that impact such code.
Yes that's a good strategy.
For example, let say I refactor something in the REST module, and realize there are some classes that shouldn't have been made public to begin with, I send a mail (possibly a VOTE?) to explain the deal and if we agree, the incriminating code is moved into internal when doing the refactoring. Of course it also means we document the breakage in the release notes
Yes, this is our current strategy: we're not allowed to add a CLIRR exclude without a VOTE. Thanks -Vincent
Now for old modules (oldcore + plugins) since we need to rewrite them IMO we just need to continue our rewriting process and when we rewrite them take that occasion to move the max stuff in the internal package.
I don't think we should touch existing oldcore/plugins code though to not break backward compat. since we need to move that code to new modules anyway and at that time we'll move old code to legacy modules.
So, unlike you, I don't think we have a major generic problem. I think we just need to be careful and make sure that committers review code committed by others and when doing this review take care to check the package. This means raising the awareness of backward-compatibility in general which is what I'm trying to achieve with the new Deprecation/Legacy policy. Note to self: need to conclude on young APIs.
Thanks -Vincent
Jerome.
[1] http://dev.xwiki.org/xwiki/bin/view/Community/JavaCodeStyle#HPackagenames [2] http://dev.xwiki.org/xwiki/bin/view/Community/DevelopmentPractices#HBackward...) [3] https://github.com/xwiki/xwiki-platform/tree/master/xwiki-platform-core/xwik...
-- Jérôme Velociter
participants (3)
-
Jerome Velociter -
Thomas Mortagne -
Vincent Massol