Re: [Banshee-List] banshee r3781 - in trunk/banshee: . build src/Core/Banshee.Services/Banshee.Playlist src/Core/Banshee.Services/Banshee.Sources src/Extensions/Banshee.Daap src/Extensions/Banshee.Daap/Banshee.Daap src/Extensions/Banshee.Daap/Daap

["Gabriel Burt" <gabriel burt gmail com>]

Hi Alex,

A couple minor and a couple major issues with this commit

1) (Major) Please, do not commit to outside of Daap or Audioscrobbler
code (eg in particular Core/) without following the patch guidelines
at http://banshee-project.org/Developers/Patches  You actually broke
playlists with this commit, and I would have caught this if we had
done a proper review.  Especially as 1.0 comes closer, we cannot
afford to have bugs like this (and more subtle) slip into Core/.

2) (Minor) Avoid using "this." superflously

3) (Major) The entire point of Mono.ZeroConf is to be an abstraction
layer around Avahi and other backends.  Please remove all
Avahi-specific code and target the MZC API only.

4) (Minor) Use playlist_id instead of playlistid.  Use track_id
instead of track, if the variable is actually the id.  Sounds like we
need to get you a copy of the Framework Design Guidelines.  There is
an online copy referenced at the bottom of HACKING you can start
reviewing for now.

Thanks for your work Alex, these pieces you're working on are important.

Gabriel