forrest-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Oshani Seneviratne" <>
Subject Re: Outline FOAF plugin
Date Thu, 07 Jun 2007 15:18:33 GMT
Hi Ross,

On 6/7/07, Ross Gardler <> wrote:
> On 06/06/07, Ross Gardler <> wrote:
> > On 06/06/07, Oshani Seneviratne <> wrote:
> > > I have created an outline FOAF plugin and attached the patch to a JIRA issue
> > > FOR-1002 [1] . I would really appreciate if my mentor/co-mentor or any one
> > > of the Forrest developers could take a look at it and comment.
> >
> > Fantastic, well done Oshani. I will have time to look over this later
> > today if Gav or someone else does not beat me to it.
> OK, Gav committed your code, here are my comments (I'm being picky,
> it's the code I'm talking about, not your work. Unless we're picky
> you'll never pick up on the best practice we have developed over the
> years. If the reasoning for something doesn't make sense please be
> sure to ask about it. Most of what we do should be sensible, but you
> may be able to help us improve these processes.

Your comments are most welcome.

> Firstly, and perhaps most importantly, you must keep status.xml up to
> date. There are a number of reasons for this:
> - it's how we generate the changes file and that's how our users
> figure out whether or not to upgrade
> - it's how you and other developers are credited with contribution to the plugin
> - it's how developers understand what is happening in the code
> - it forms part of the documentation (especially at release time)

Thanks for this advice. Looking at the the status.xml files elsewhere
and from the documentation I knew that I need to update it. But I
deferred it altogether until I get the plugin to work.  And
unfortunately forgot to make the changes before submitting.

I assure you that, in my next patch you will see things other than my
name in the status.xml :).

> Secondly, I'm afraid I couldn't get it to work. First off, your
> file indicated that the dispatcher plugins were to
> be used, yet there was no definition of the dispatcher theme to be
> used. you mention that you had problems with dispatcher and were now
> working with the dispatcher. So I removed these entries.
> I then found that there was a full duplication of all content in most
> files. Looking at the patch you supplied this duplication was created
> when you created the diff. I'm not sure how you managed this, I've
> never seen it before. But you should get into the habit of reading the
> diff before submitting it (and to be fair Gav made a mistake in
> committing it without reviewing it properly - that's our job as
> committers).
> I went through and tidied these files up and committed them.
> In summary - more attention to detail before submitting would save
> other committers plenty of time.

Just now I thoroughly examined the original patch I submitted
yesterday, and I see what you mean. I wish I had done that yesterday
itself, instead of giving it a cursory glance just to see whether all
the files were there. I was only worried about any missing files, and
was not at all concerned about checking for duplicates!

FWIW, I used TortoiseSVN to create the patch, and I trust it do the
job well because I have not had any problems in creating patches with
it in the past. So in this case, these duplicates are a real mystery
to me too. I am 100% sure that I never added these files twice before
creating the diff.

Anyway, I am really sorry for the extra trouble and the time you
wasted on going through my patch. So, next time I would be extra more
careful before submitting a patch. I will definitely double check
everything and may be even apply it to a fresh checkout to see if it
is alright and ready for submission.

> You should keep a sharp eye on commits to the DOAP plugin. I'll be
> making some core design changes when I get the time to do so. Whenever
> you feel these changes are applicable to your FOAF plugin you should
> follow these changes. The changes are based on experience using the
> DOAP plugin in production. If you think they are a bad move then
> please speak up.

Yes, I will definitely do that. Even for this initial version of the
FOAF plugin, I learnt quite a lot from the DOAP plugin.

> Now, to your specific questions...
> > > So, my question is, is this the expected way of letting the forrest build
> > > know about a new plugin (at least until the plugin is registered in the
> > > official plugin descriptor files)?
> >
> > That is an oversight in the documentation, it would be good if you
> > could provide a patch to the docs.
> >
> > In this case you can add your entry directly to the whiteboard
> > plugins.xml file as we will certainly be accepting your plugin into
> > the whiteboard.
> I've committed this to SVN. It would be really good if you could
> clarify the documentation for us.

Glad to do it. Hope changes to the
would do the trick.

> (are you subscribed to the svn@forrest? [4] you should be so that you
> can see updates we make to your code)

Yes, I got myself subscribed today.

> > > 2. When I ran the 'ant test' target against the plugin, it failed saying
> > > that there are broken links in the site. I found the culprit to be a couple
> > > of @rdf:ID and  @rdf:resource elements I embedded in the xsl.
> > >
> > >  For example,  there's the following line in the foaf-to-document.xsl
> > >  <a href="{foaf:homepage/@rdf:resource}"><xsl:value-of
> > > select="foaf:homepage/@rdf:resource"/></a>
> > >
> > >  When Forrest renders this link it leads to a relative path like
> > > "http://localhost:8888/homepage_url_given_in_foaf:homepage/@rdf:resource".
> > >  How can we make it absolute, so that it will be linked to
> > > "homepage_url_given_in_foaf:homepage/@rdf:resource" and not
> > > the relative url as in above?
> >
> > I'll need to look at this in the code to see exactly what is
> > happening, I understand the question, but not the context at present.
> > perhaps you should raise an issue about this. I just added a new
> > "Plugin: input.FOAF" component to JIRA for you.
> OK, I see the problem now:
> <foaf:schoolHomepage rdf:resource=""/>
> This is not valid as far as I am aware. I believe an rdf:resource must
> be a URI, which must include a scheme (i.e. http) (I'm no RDF expert
> though). See [6]

Yes, you are absolutely right. Thanks a lot for finding this mistake!
Too bad that the W3C RDF validator [7] didn't report this error.
Anyway, it was my mistake to have included it in the example FOAF file
in the first place.

> > > 3.I was able to see the transformation I intended with the xsl at
> > > http://localhost:8888/personDetails.xml (in one of these
> > > forrest's internal doc forms I believe). However, when I point my browser to
> > > http://localhost:8888/personDetails.html, I could only see
> > > a blank page with the default forrest skin applied. The content was missing!
> > > Now, how is this page generated? Is the
> > > FORREST_HOME/main/webapp/sitemap.xmap responsible for
> > > generating this html page?
> >
> > Yes. The document at [3] should help understand the process, in
> > particular see the section "Understanding the HTML-Pipeline"
> >
> > > More importantly, what changes should I make to get those content from the
> > > transformed FOAF into that html?
> First observation is that your XDoc returned by
> http://localhost:8888/personDetails.xml is not a valid XDoc file. For
> example, it includes <h1> elements which are not valid - see [5].
> Secondly, it is given the namespace of,
> which is obviously incorrect. This is because your XSL uses a defaul
> XHTML namespace.

Thanks again for spotting these!
After removing the XHTML namespace the page was rendered properly.
<h1>s and <h2>s didn't seem to do any harm. Anyway, I will get rid of
those and use appropriate elements in their place to make it a valid

Thanks & Regards,

> > >  [1]
> > > [2]
> > >
> > >
> >
> > [3]
> [4]
> [5]
> [6]


View raw message