On May 22, 2012, at 12:06 PM, Jerome Velociter wrote:
On Tue, May 22, 2012 at 11:50 AM, Vincent Massol
<vincent(a)massol.net> 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#HBackwar…)
>> [3]
https://github.com/xwiki/xwiki-platform/tree/master/xwiki-platform-core/xwi…
>>
>> --
>> Jérôme Velociter