12 Feb
2014
12 Feb
'14
10:34 a.m.
Hi Denis,
On 12 Feb 2014 at 02:34:13, Denis Gervalle (dgl@softec.lu(mailto:dgl@softec.lu)) wrote:
Hi Vincent,
I have the exact same reading then Thomas. The @since tag should not be use
for anything else than what it is made for: notifying the user of any
package/class/method at which point it has been introduced. A
package/class/method is define by its canonical name, and any change in it
is therefore another package/class/method, even if it has the exact same
signature.
While I understand that this could have not been properly followed in the
past, this is clearly a mistake, and it is mostly due to the lack of
automation/check on these annotations.
Ok, let’s put this part behind us since we agree :) I’m going to add the following to our
dev practices document, let me know if you guys are ok:
------
= @Since tag practices =
We follow the practices defined in the [[official JavaDoc
guide>>http://www.oracle.com/technetwork/java/javase/documentation/in…ince]].
Specifically, the @since tag should be used in cases both when a new type is introduced
(class, interface, enum, method, public class field, etc) and when a type is
moved/renamed.
Also note that when a type is moved to another package if it already had a @since tag,
it’s required to update it with the new version since the FQN has changed.
The preferred format for common @since messages are listed below.
|=Reason|=@since Description
|Renamed a former method that’s been deprecated|@since <version>, rename of {@link
#someMethod()}
|Renamed a former type that’s not been deprecated (young API)|@since <version>,
rename of {@code <old FQN of type>}
|Generalization of method|@since <version>, replaced {@link #someMethod()}
|New type|@since <version>
------
(inspired
from http://www.liferay.com/community/wiki/-/wiki/Main/Javadoc+Guidelines#sectio…).
The removal of the @Unstable annotation has nothing to
do with @since, and
should not be mixed up.
I found very annoying to see the need to know for sure the introduction
time of the @Unstable annotation, simple to respect a rules we have only
voted to avoid those @Unstable to be left for ever. Normally, the "owner"
of the newly introduced API, should take care to remove those @Unstable as
earlier as possible, and the maximum should only be reach in very rare
cases. So, my feeling is that those rare case could be solved using Git
history/blame if ever needed.
Why do you say it’s rare? Nobody remembers to remove @unstable, me being the first. I need
a reminder of some sort. And even if it was me removing them without a reminder, I'd
still need to remember when I introduced a new API...
So I am not in favor of any of your "new"
options. I would simply clarify
the rules for @since (if really needed for those not understanding the
obvious). I also remind all committers that it is your responsibility to
remove the @Unstable you have introduce in due time.
This is just wishful thinking and will never happen. We need some process to ensure this
either automated or manual.
It’s like saying that it’s everyone’s responsibility to document the new features he
introduces in the release notes and on xwiki.org. Do you think this worked by itself and
that all devs did it by themselves? It didn’t happen for 5 years and it wouldn’t have for
the following 10 years if we hadn’t set up a reminder. And the reminder is that when we
release we check jira for undocumented issues. This 1 year cycle reminder for @unstable is
exactly the same. If we knew how to automate it quickly we would do it, except that we
don’t so we have to set up a reminder.
Note that I did all the work of checking and you’re still complaining :) I find that
pretty unfair! Note that I raised the parts of the code where @unstable had to be removed
and ATM I’m still the only one to have removed those tags but that’s probably because devs
are still busy on 5.4.x and didn’t get the time to look at 6.0.
If you really want to add new annotations, I would be
far more interested
to distinguish API and SPI, and provide them a different stability,
allowing us more flexibility :)
1) Even if we were distinguishing API and SPI we would still need to be able to remove
@unstable tags at least for the API part so it doesn’t change anything to the current
discussion.
2) We started discussing API and SPI on the list and the main issue is that xwiki is a dev
platform and it’s hard to make a difference between an API and a SPI. The only one that
makes sense to me is a difference between a Scripting API from a Java API (instead of API
vs SPI) and I’d be willing to define different rules for both. But we even discussed that
and said we wanted XWiki to be a dev platform and not just an Application, and if you
consider it like a dev platform then you need stable APIs for everything (same as with a
dev language, imagine Java breaking stuff in each release, do you think it would have been
successful?). If we want to encourage people to develop against our Java API we need some
stability. Now we could also say that we don’t want that anymore and that not a lot of
people are coding against our Java API and thus it doesn’t matter at this stage if we
break them since there are only a few affected persons. I’d be ok with that provided we
preserve the scripting APIs. We would need to handle the Scripting API requiring PR though
and decide if it’s ok to break them. It would have to be ok as otherwise you can access
the full XWiki Java API from those… This means we would break a certain amount of
extensions since quite a few do use protected scripting API as they need it and don’t have
equivalent safe APIs.
Generally speaking, I’m fine with setting up a different process for handling young apis
but you have to propose something that can work. Just proposing something that is sure to
fail will not help, it’ll just push stuff under the rug.
Now, are you willing to spend time writing an automated check when @Unstable need to be
removed? It can be done for sure, it could be something like this:
- use checkstyle’s java code parser and write a checkstyle rule to find all @unstable tags
in javadoc (easy difficulty)
- for each @unstable found, use Git to find out when it was introduced. There are some
potential issues with this since the code needs to be careful to follow renames for
example (medium difficulty)
- then map the date with the XWiki version (hard difficulty since you need to maintain a
map or query xwiki.org, or easy difficulty if we use @Unstable(“5.4M1”) since we have the
XWiki version in the tag itself and the previous step isn’t needed anymore and only the
following step is required)
- extract the current version from the pom.xml and report an error if @unstable has to be
removed
Thanks
-Vincent
Thanks,
On Tue, Feb 11, 2014 at 3:22 PM, Thomas Mortagne
wrote:
> On Tue, Feb 11, 2014 at 3:19 PM, Thomas Mortagne
> wrote:
> > On Tue, Feb 11, 2014 at 3:15 PM, vincent(a)massol.net
> wrote:
> >>
> >> See below.
> >>
> >> On 11 Feb 2014 at 13:17:03, Marius Dumitru Florea (
> mariusdumitru.florea@xwiki.com(mailto:mariusdumitru.florea@xwiki.com))
> wrote:
> >>
> >>> I agree with Thomas and Denis, but I must admit that I haven't
updated
> >>> the @since version when I did refactorings in the past. I'll pay
> >>> attention to this next time.
> >>>
> >>> Thanks,
> >>> Marius
> >>>
> >>> On Mon, Feb 10, 2014 at 2:51 PM, Denis Gervalle wrote:
> >>> > On Sun, Feb 9, 2014 at 11:16 PM, Thomas Mortagne
> >>> > wrote:
> >>> >
> >>> >> On Sun, Feb 9, 2014 at 6:10 PM, vincent(a)massol.net
> >>> >> wrote:
> >>> >> > Hi devs,
> >>> >> >
> >>> >> > I always ask myself this question so I think we need a
common
> agreement.
> >>> >> >
> >>> >> > So here's the question:
> >>> >> > * I have added some code in version N and this I have a
"@since
> N" in
> >>> >> the code
> >>> >> > * In version M (M > N), I move the class/interface to
a new
> package
> >>> >> >
> >>> >> > Question: Do I change the @since annotation to
"@since M" or not?
> >>> >> >
> >>> >> > 2 possibilities:
> >>> >> > * Reasoning 1: it's a new class/interface since the
FQN of the
> >>> >> class/interface has changed and thus we should use
"@since M"
> >>> >> > * Reasoning 2: even though the FQN has changed it's
still the
> same code
> >>> >> that was moved and from a user POV, it was still introduced
in
> version N
> >>> >> and thus we should keep "@since N"
> >>> >> >
> >>> >> > WDYT?
> >>> >> >
> >>> >> > I'm hesitating. The most technically correct answer
is Reasoning
> 1 IMO
> >>> >> but the most useful one is probably Reasoning 2 since the
question
> we wish
> >>> >> to answer is probably: "when was this code first
introduced?".
> >>> >> >
> >>> >> > Thus reasoning 2 seems slightly better to me.
> >>> >>
> >>> >> Big -1 for 2 which is totally out of context, @since indicate
that
> you
> >>> >> can use that class or method since that version in you code
and
> >>> >> indicate you which version you are going to be compatible
with. If
> you
> >>> >> change the class or method your can't keep the same
@since. If you
> >>> >> want to know since when the feature exist look at
xwiki.org...
> >>> >>
> >>> >
> >>> > I completely agree with Thomas, a -1 for 2)
> >>> > I would add that if you want to know from where the code come
from,
> Git is
> >>> > your best friend.
> >>
> >>
> >>
> >> I don't fully agree with this.
> >>
> >> The point of the @since tag is exactly to NOT have to check in Git to
> see when some code was introduced! And with your logic, the @since tag is
> never needed at all since we can always check in Git, and it's as easy to
> check in Git for Reasoning 1 than it is for Reasoning 2.
> >
> > The point of @since is to indicate since when a signature exist which
> > means that 2 is completely wrong. I never talked about git, I don't
> > care how you know since when a feature exist but please don't use
> > @since which has a different meaning for that.
> >
> >>
> >> If you start changing the @since then it makes it impossible to
> properly remove the @Unstable annotations later on since for each @Unstable
> annotation you'll need to do some deep Git archeology to reconstruct the
> first time the API was introduced.
> >>
> >> Also, I can tell you that a lot of devs (the majority, if not 80%) have
> been doing Reasoning 2 since the beginning of our usage of @since, since
> it's the simplest thing to do and it's what you get by default if you
don't
> do anything... I know I did it, I know Marius did too and I'm pretty sure
> others too.
> >>
> >> So to recap, my points are:
> >> * If you need to find out when some class was moved in another package
> you can always check Git and you don't need the @since that for this
> >> * Reasoning 1 makes it almost impossible in practice to remove
> @Unstable annotations
> >> * Reasoning 2 is complex to implement (the proof being that for most of
> our code it wasn't done)
> >>
> >>
> >>
> >> Note that this discussion is important since we never formalized how to
> use the @since annotation (it's not documented on
> http://dev.xwiki.org/xwiki/bin/view/Community/DevelopmentPractices).
> >>
> >> Also note that
>
http://www.oracle.com/technetwork/java/javase/documentation/index-137868.ht…
explain what to do when a class/interface is moved elsewhere. I
> googled and couldn't find how other projects handle the since tag.
>
> It does no say it because it's obvious, if you change the package you
> change the name, the class short name does not have any value in Java.
>
> >>
> >> So taking everything into account we have the following options:
> >>
> >> A) Update the @since value when we move a class/interface to another
> package. But force the @Unstable annotation to have a text specified (ATM
> it's optional) with the rule of specifying when the API is introduced. For
> example @Unstable("Introduced in 5.4M1").
> >>
> >> B) Keep what we've been doing implicitly, which is to not change the
> @since value when a class/interface is moved to another package and
> consider that this @since tag corresponds to when the code was first
> introduced independently of its class/interface location. In this case no
> need to use a text for the @Unstable annotation.
> >>
> >> C) Use some other annotation like for example @Introduced("5.2")
or
> @Introduced in 5.2 (javadoc).
> >>
> >> As for automating the addition of the since tags, I couldn't find
> anything good for us to use. FTR I found:
> >> -
>
http://stackoverflow.com/questions/3417243/automatic-since-javadoc-tag-for-… the
maven plugin doesn't do magic.
> >> -
>
http://help.eclipse.org/indigo/index.jsp?topic=%2Forg.eclipse.pde.doc.user%…
> >>
> >> While I prefer A) which I find more technically correct I think it's
> also a lot more work to enforce (in lots of places it means duplicating
> information between @since and @unstable) so I'm hesitating, especially
> since we've been doing B implicitly.
> >>
> >> Any idea/preference?
> >>
> >> Thanks
> >> -Vincent
> >>
> >>
> >>> > I take the occasion to also mention that it would be nice to have
a
> better
> >>> > way to maintain those @since. At least a check of presence, or
even
> better
> >>> > a check of correctness, in the build would nice to have. The must
> being to
> >>> > have those @since added automagically :)
> >>> >
> >>> >
> >>> >>
> >>> >> >
> >>> >> > Thanks
> >>> >> > -Vincent