Re: [Tracker] Anonymous File Nodes branch review

[Martyn Russell <martyn lanedo com>]

On 03/02/10 13:05, Carlos Garnacho wrote:
> > 1. We should use G_DIR_SEPARATOR_S here instead:
> >
> > +       slash_uri = g_strconcat (source_uri, "/", NULL);
>
> Hmm, we're building an uri here, I don't think we want uris with '\' if
> tracker is ported (again?) to Windows :)

This is a good point :P

> > 2. This looks like some left over crack that needs fixing :)
> >
> > -       tracker_sparql_builder_object_iri (sparql, uri);
> > +       tracker_sparql_builder_object (sparql, "_:foo");
>
> I guess _:foo can be renamed to something more descriptive, but that's
> precisely the way to create autogenerated urns.

Just to repeat what we decided on IRC, we can use _:file that makes more 
sense.

> > 4. Don't we need quotes round ?f (in add_tag_for_urns and again in
> > remove_tag_for_urns)?
> >
> > -                                        "  ?urn nie:isStoredAs ?f ."
> > +                                        "  ?urn nie:url ?f ."
>
> not AFAIK, if we used a literal there such as \"%s\" we'd need these.

Ah yea, of course.

> > 5. I would improve the this so the query is more readable personally:
> >
> > stmt = tracker_db_interface_create_statement (iface, "SELECT ID FROM
> > \"nie:DataObject\" WHERE \"nie:DataObject\".\"nie:url\" = ?")
>
> hmm? has that changed in this branch?

Ignore me on this, I thought it was SPARQL. This is one of Philip's changes.

> > 13. Hmm, is this correct in extract_html(), extract_imagemagick(), etc:
> >
> > -       tracker_sparql_builder_subject_iri (metadata, uri);
>
> Yes it is, metadata here is now an embedded TrackerSparqlBuilder, and
> the subject must be set by tracker-extract.c so it's the same than the
> one used in tracker-miner-fs, so extractors now must not specify
> subjects for the embedded metadata.

Aha, I thought this might be the case, just wanted to confirm really.

--
Regards,
Martyn