You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fop-dev@xmlgraphics.apache.org by Jeremias Maerki <de...@jeremias-maerki.ch> on 2010/09/02 12:04:55 UTC

Re: TODO tag [was: Re: svn commit: r990148 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: area/ fo/ fo/flow/ fo/flow/table/ fo/pagination/ fo/properties/ hyphenation/ layoutmgr/ layoutmgr/inline/ layoutmgr/table/]

Just to add my 0.05 CHF: I normally use @todo when adding TODOs as part
of a method or class comment and I use //TODO inline. That's because
plain "TODO" lets the full text bleed into the Javadocs which is
certainly something we don't want. See for example:
org.apache.fop.area.DestinationData.resolveIDRef()

------
public void resolveIDRef(java.lang.String id,
                         java.util.List pages)

    Resolves the idref of this object by getting the PageViewport object that corresponds to the IDRef This method allows the Resolvable object to resolve one of its unresolved idrefs with the actual set of PageViewports containing the target ID. The Resolvable object initially identifies to the AreaTreeHandler which idrefs it needs resolved. After the idrefs are resolved, the ATH calls this method to allow the Resolvable object to update itself with the PageViewport information. List) TODO check to make sure it works if multiple bookmark-items have the same idref

    Specified by:
        resolveIDRef in interface Resolvable

    Parameters:
        id - an ID matching one of the Resolvable object's unresolved idref's.
        pages - the list of PageViewports with the given ID
------

Or in org.apache.xmlgraphics.fonts.Glyphs:

------
public static final java.lang.String glyphToString(java.lang.String name)

    Deprecated. User getUnicodeCodePointsForGlyphName instead. This method only returns the first Unicode code point it finds.

    Return the glyphname from a string, eg, glyphToString("\\") returns "backslash"

    Parameters:
        name - glyph to evaluate 
    Returns:
        the name of the glyph TODO: javadocs for glyphToString and stringToGlyph are confused
------

I think we may still have various occurences where an empty line instead
of a <br> or <p> was used to delimit a paragraph which sometimes leads
to odd looking Javadocs. (but that's a different topic)

I'm not particularly in favor of @asf.todo because I'm sure most people
will instinctively rather use @todo and have to make an effort to
remember @asf.todo. I'm not sure we have the tooling to make sure noone
uses @todo.

I've checked a few other ASF projects' source code to see what they do.
A few have "TODO" in their Javadocs comments with the side-effect
described above. Others, like Felix, don't put their TODO's in the
Javadoc comment at all but use this pattern:

------
/**
 * This class caches conditions instances by their infos. Furthermore, it allows
 * to eval postponed condition permission tuples as per spec (see 9.45).
 */
// TODO: maybe use bundle events instead of soft/weak references.
public final class Conditions
------

That would also be safe. Actual Javadoc comments like @todo are
relatively few.

Based on these findings, I recommend we eventually remove TODOs and
@todo from the Javadoc comments and use //TODO comments exclusively.
Eclipse can handle all TODOs correctly by default. And Javadocs don't
get polluted. I believe that would pretty much address everyone's
concerns.


On 31.08.2010 13:33:25 Vincent Hennebert wrote:
> Hi,
> 
> I just thought I would homogenize our usage of todo tags and match what
> seems to be the de facto standard (“TODO”) among current committers.
> Most @todo indeed come from very old commits. I didn’t realise that
> javadoc could do something with them, which is why that looked to me
> like a minor change that wasn’t needing prior discussion. Sorry about
> that.
> 
> Ok, so there is something that can be done out of @todo tags in javadoc
> comments. Now, having to use our own namespaced version is unfortunate
> and looks overkill to me. Just to have a slightly better formatted
> javadoc? Are such comments of any use to users of the API anyway? Most
> of them rather look like pure internal development issues and should
> probably not even appear in the javadoc.
> 
> Also, while @todo tags can be indexed, modern IDEs can index plain TODO
> tokens as well, so that reduces the advantage of @asf.todo IMO.
> 
> If there are strong feelings against the removal of @asf.todo, I’ll
> revert the change. Otherwise, I’ll actually complete it by removing the
> definition of the custom tag in build.xml, which I hadn’t spotted.
> 
> Vincent
> 
> 
> Simon Pepping wrote:
> > It would indeed have been better to first have a discussion and then
> > make the change. @asf.todo is specific enough that we could have
> > changed it at any time. That said, Glenn's change was also made
> > without a discussion. My javadoc does not complain about the @todo
> > tag, and I had not understood that this was a motivation.
> > 
> > The javadoc documentation (of my sun-java6-jdk) is not clear about
> > this topic, and uses @todo liberally in its section about the -tag
> > option. Its most informative paragraph is this:
> > 
> > "Avoiding Conflicts - If you want to slice out your own namespace, you
> > can use a dot-separated naming convention similar to that used for
> > packages: com.mycompany.todo. Sun will continue to create standard
> > tags whose names do not contain dots. Any tag you create will override
> > the behavior of a tag by the same name defined by Sun. In other words,
> > if you create a tag or taglet @todo, it will always have the same
> > behavior you define, even if Sun later creates a standard tag of the
> > same name."
> > 
> > which does not even go so far as to discourage the @todo tag. It is
> > also not clear how a todo tag would be a specific asf tag, different
> > from the todo tag of any other organization. Everybody uses todo and
> > means the same with it.
> > 
> > Using the widely recognized TODO keyword circumvents the tag question
> > altogether, but is outdated since the advent of tags.
> > 
> > Let us discuss this and not waste effort on undoing each other's
> > expression of their point of view. Let us also not forget that working
> > in a team requires compromises; the code will never match your own
> > conventions and preferences as precisely as code in your very own
> > project. This is more so in an open project with a long history and a
> > large set of authors.
> > 
> > Simon
> > 
> > On Sat, Aug 28, 2010 at 09:28:06AM +0800, Glenn Adams wrote:
> >> Vincent,
> >>
> >> Could you explain your rationale for this change? Originally, these were all
> >> marked with a non-standard '@todo' javadoc tag, which javadoc complained
> >> about, indicating that for "non-standard" tags, there should be at least one
> >> '.' present in the tag name. I had fixed this by adding the "asf." prefix,
> >> which still allowed tracking these in javadoc more easily. However, your
> >> change now removes the utility of the tag.
> >>
> >> On a more general point, wouldn't it be more useful to have a discussion
> >> about stylistic changes prior to implementing them? Just so we can get on
> >> the same page?
> >>
> >> Regards,
> >> Glenn
> >>
> >> On Fri, Aug 27, 2010 at 9:31 PM, <vh...@apache.org> wrote:
> >>
> >>> Author: vhennebert
> >>> Date: Fri Aug 27 13:31:41 2010
> >>> New Revision: 990148
> >>>
> >>> URL: http://svn.apache.org/viewvc?rev=990148&view=rev
> >>> Log:
> >>> Replaced @asf.todo with normal TODO comment
> >>>
> >>>
> > 




Jeremias Maerki


Re: TODO tag [was: Re: svn commit: r990148 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: area/ fo/ fo/flow/ fo/flow/table/ fo/pagination/ fo/properties/ hyphenation/ layoutmgr/ layoutmgr/inline/ layoutmgr/table/]

Posted by "J.Pietschmann" <j3...@yahoo.de>.
On 02.09.2010 12:14, Glenn Adams wrote:
> also the doclet options to permit use of @todo without warnings. I could try
> to experiment some to see if that is feasible, then we could return to the
> former usage of @todo.

Javadoc 1.5 or later can be passed a command line option defining 
additional tokens to accept:
 
http://download.oracle.com/javase/1.5.0/docs/tooldocs/windows/javadoc.html#tag

I vaguely remember to have had it working in my local build.xml some
times ago.

J.Pietschmann

Re: TODO tag [was: Re: svn commit: r990148 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: area/ fo/ fo/flow/ fo/flow/table/ fo/pagination/ fo/properties/ hyphenation/ layoutmgr/ layoutmgr/inline/ layoutmgr/table/]

Posted by Glenn Adams <gl...@skynav.com>.
On Thu, Sep 2, 2010 at 6:04 PM, Jeremias Maerki <de...@jeremias-maerki.ch>wrote:

> I'm not sure we have the tooling to make sure noone
> uses @todo.
>

Actually, checkstyle 5.1 will report warnings for any use of a non-standard
tag that is not qualified with a dotted prefix. Also the standard Doclet in
recent JDKs will complain as well. So if committers run checkstyle and
javadocs targets before committing, we should be able to keep this usage
out.

On the other hand, it may be possible to fine tune the checkstyle rules and
also the doclet options to permit use of @todo without warnings. I could try
to experiment some to see if that is feasible, then we could return to the
former usage of @todo.

G.