Re: [Rhythmbox-devel] unreviewed patches



On Thu, 2005-08-11 at 17:05 -0400, Colin Walters wrote:
> Not sure if everyone knows about this page but I didn't:
> 
> http://bugzilla.gnome.org/reports/patch-report.cgi?product=rhythmbox

I found that about a month ago, it's handy for seeing which patches need
commenting on/comitting. Below is my opinion of the patches listed:


Bug 110928 - Audio CD support:
Due to a typo on my part, the attached patch doesn't actually compile,
although it's a 1 line fix. I'm planning to do some more work on this
over the next few days to clean up the "removable media" part and squash
some bugs. I haven't thought much about metadata lookup yet, but we can
probably steal that from Sound Juicer.


Bug 124828 - Making "rhythmbox --play" work better:
Sounds good - although as per my last comment on the bug, I still can't
get it to work for playlists.


Bug 124828 - More auto playlist criteria:
This was in the merge branch, so hopefully most of the bug got ironed
out. I don't know what the general opinion on how I've moved criteria
related bits around is, but it should make it easier to add new ones in
the future.


Bug 141811 - add playcount criteria to auto playlist:
This is basically obsoleted by my patch on bug 124828


Bug 149716 - add iRiver player support:
I haven't checked whether it applies cleanly to cvs, but getting support
for more mp3 players would be good. If my "removable media" stuff from
110928 is acceptable (once I've cleaned it up) it's probably worth using
that as a base for iRiver support, so we don't have all the code
duplication.


Bug 158599 - Natural sort order:
Would be good to do. Jonathan's patch works fine for albums - I'm not
sure whether doing it like this is noticeably slower than when using the
sort keys returned by g_utf8_collate or not.

Glib 2.7 has g_utf8_collate_key_for_filename(), which does exactly what
we want, but it's only in 2.7 and we depend on 2.6.


Bug 170057 - FHS compliance in iPod support:
Looks fine, but we *really* need to fix HAL support, so that it works
with HAL >= 0.5.0 (bug 310017)


Bug 309696 - start playing when "rhythmbox file" is run:
Currently not all sources implement the search method, which means that
RB will crash when trying to do this with such a source is selected.
Maybe we should make rb_source_search skip the call if
rb_source_can_search returns false.

Also what should happen if the currently selected source doesn't contain
the file, after it has been added - maybe switching to the library
source?


Bug 309817 - Use new Playlist parser features:
Basically just needs finishing.


Bug 310017 - RB doesn't compile with newer HAL (>= 0.5):
The attached patch makes it work, but drops support for HAL < 0.5, we
should probably check at compile time and have both work.


Bug 97500 - Rhythmbox doesn't support remotes with one button for track
skip and seeking:
IIRC lirc support is now an external app, that talks to RB via
bonobo/dbus. So is this obsolete?


Bug 128110 - Enter in search field should move focus to song list:
Sounds good to me.


Bug 131544 - A few ideas to improve searches:
One problem with this is that you can no longer use the browsers and
search box at the same time. I do use that functionality a bit, e.g.
selecting a genre and then typing in the search box. I'm not sure of the
best solution here.


Bug 139102 - Desensitise the slider rather than hiding it:
Works for me, and mentions that it's more HIG compliant.


Bug 161935 - show source list when creating new playlist:
Makes sense to me.


Bug 300867 - use BaconVolume widget:
Looks okay, although it isn't obvious that the widget is clickable (no
border around it).


Bug 309947 - Converting filenames doesn't work as expected:
I don't know a huge amount about Unicode and locale conversion, but
Christophe seems to have issues with what it's trying to do.


Bug 311199 - show disc number in entry view and song properties:
I agree with Christophe that using "d.t" (d=disc number, t=track number)
doesn't look great, but I'm not sure how best to do this.


Bug 312122 - disable autosizing of entry view columns:
Jonathan did some profiling and the column autosizing is massively
slowing down displaying the results of queries. This makes it faster,
and I haven't notice any bad side-effects.


Hopefully we'll get some of these in cvs soon, so they can get more
widespread testing.


Cheers,

James "Doc" Livingston 
-- 
"Zero Tolerance" in this case meaning "We're too stupid to be able to
apply conscious thought on a case-by-case basis". -- Mike Sphar

Attachment: signature.asc
Description: This is a digitally signed message part



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]