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#HBackwardCompatibility) >>> [3] >>> https://github.com/xwiki/xwiki-platform/tree/master/xwiki-platform-core/xwiki-platform-wysiwyg/xwiki-platform-wysiwyg-client/src/main/java/org/xwiki/gwt/wysiwyg/client/ >>> >>> -- >>> Jérôme Velociter _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
