Hi Vincent,
On Wed, Feb 12, 2014 at 10:34 AM, vincent(a)massol.net <vincent(a)massol.net>wrote;wrote:
  Hi Denis,
 On 12 Feb 2014 at 02:34:13, Denis Gervalle (dgl(a)softec.lu(mailto:
 dgl(a)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/index-137868.ht…]
 ].
 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…
 ).
 
+1
   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? 
 
I have not said it is rare, but that it should be 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...
 
You should follow the rules we defined for @Unstable, else it is useless to
vote them:
"However, the previous rule [ed: removal after a maximum of 1 full] is only
a maximum and developers are encouraged to remove the unstable annotation
whenever they feel that the API should come out of unstability. When this
happens the standard deprecation mechanism then kicks in."
This is an important rule. If we forgot about the @Unstable we have
introduced, it means we do not use it right. Keeping stuff that we do not
work on in an unstable state is useless, and does not help our users. If we
forgot what you are working on, I am worried.
  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.
 
 
While I understand your point, I am still really annoyed by this way of
thinking. @Unstable should only be used when you introduce new stuffs, and
until you remove it, it is almost like you have not really add it for our
users. It looks like unfinished work, that should still have at least a
corresponding issue open. Announcing new @Unstable API in the RN is closing
one issue, but opening another one which will lead to announce the same new
API as stable at some later point. If you cannot open that second issue,
why do you keep the API @Unstable, what do you fear (except the SPI aspect,
see below) ?
  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.
 
Where did you see me complaining ? I have just said that what you had need
to do should not have been in first place, except in rare cases.
   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.
 
 
Not exactly, it would be easier to remove early the @Unstable annotation on
API interfaces. Stabilizing an SPI is a bigger commitment, and require more
time.
 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?). 
In Java Platform, there are some distinctions between API and SPI as well,
simply look at the spi packages, the Spi suffixed interface name, ....
  If we want to encourage people to develop against our
Java API we need
 some stability. 
An API could be stable and be improved by adding new methods and
deprecating older one, while an SPI cannot and require a separate
additional interface, this is what I am talking about.
  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. 
I do not see it that way. Declaring our interface as an API or an SPI would
be more a declaration of our intent and our commitment. It is like when we
put public stuff in an internal package, you may still use it externally,
but you know the risks. Distinguishing would only help users implementing
an interface, not users simply using it.
For an interface declared API (the default, no annotation required), we
declare that we may freely add new methods, when an interface is declared
SPI, it is guaranteed to be left unchanged definitely. This does not
prevent you to implement an API, but you know the risks. The benefit for us
would be an increased flexibility when we evolved an API. It would reduced
the need for a complete replacement, allowing a more smooth evolution of
our API.
Choosing is fairly simple for me. By default, all interfaces are API. Only
a few of our existing public interface will be declared SPI, and we will
ensure those will be frozen definitively. Which one ? The one we have
precisely design to be also implemented by a user. Once again, it does not
prevent a user to implement an API, but he will know that it should be
prepare to a possible breakage.
Let me take a very simple concrete example in my domain, the security
authentication module. The AuthorizationManager interface is an API, we do
not expect users to implement it differently most of the time. But the
AuthorizationSettler interface has been designed for and will be an SPI.
Users are encouraged to implement their own AuthorizationSettler if they
want to customize the behavior of the AuthorizationManager. Even more
typical, the RightDescription interface is only an SPI, since we never
implement it.
  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.
 
Introducing more flexibility in our backward compatibility promises is IMO
a way to reduce as much as possible the length of time we keep the
@Unstable annotation. Reducing the usage of @Unstable is beneficial to our
users, and also to us.
 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
 
If we have the above need, I prefer we simply stop using the @Unstable
annotation. It became counter productive for our users. If you have time
for such automation, use it to automate the @since to make it more
accurate, which will be far more helpful to us and our users.
Thanks,
 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
 > 
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-…
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 
 _______________________________________________
 devs mailing list
 devs(a)xwiki.org
 
http://lists.xwiki.org/mailman/listinfo/devs
 
--
Denis Gervalle
SOFTEC sa - CEO