Re: Some code changes wanted in Rygel

[Vincent Untz <vuntz gnome org>]

Le lundi 27 septembre 2010, à 16:27 +0300, Zeeshan Ali (Khattak) a écrit :
> Hi,
> 
> On Mon, Sep 27, 2010 at 2:30 PM, Vincent Untz <vuntz gnome org> wrote:
> > What are
> >> the risks involved in approving this patch?
> 
>   None that I can imagine for the changes in 8c0391d..14071c0.
> 
> >  - there's at least one unrelated fix ("media-export: Add null checks to
> >   standard XDG folders")
> 
>   There was a bug about this on debian bugzilla. Now that you mention
> it, this shouldn't matter to a normal GNOME desktop but rygel is used
> by people on headless machines where this issue will realize.

I'm sorry if this sounds like a lot of paperwork, but this is another
issue, so it's better for us to track this in a separate thread -- it's
already a bit confusing right now, so adding something else on top of
that won't help. And there is a question to start this thread: what does
it do with NULL? Does it crash?

> > My understanding is that the first three commits go together. How
> > critical are those changes?
> 
>   The first one (206f019) fixes a leak introduced in previous release.
> 8c0391d..14071c0 are the fixes for IOP.

Ah, I hadn't see this one due to the way git log works (see below).

I'm confused: the commit says "Don't set description if already set",
but you always do a set_content(). I don't know if this is what you
wanted?

(Else, I guess the old code was adding a second description child to the
XML; that part is okay)

> > How many devices will fail to work with
> > rygel?
> 
>   AFAICT 3 different commercial products, however I can only be 100%
> certain about one of them since the other two bug reporters haven't
> managed to verify the bugs yet.
> 
>   So I modify my request to only include the following commits:
> 
> 206f019..f6bfed4, 42a0f7b and f0b0127.

So, for people using git log, you want to do git log on
206f019^..f6bfed4, I guess.

For 206f019, see above.

For the rest of 206f019..f6bfed4, I think I'm okay with it, but the
renames make it hard to see if there was any change in the xml files.
Even with git diff -M :/ So I'll give a first approval.

42a0f7b is "Add null checks to standard XDG folders". Let's use a new
thread for this one, and see my question above

I already gave +1 for f0b0127. Missing a second approval, I guess.

Vincent

-- 
Les gens heureux ne sont pas pressés.