Sorry for the slow reply... a deadline at work meant I haven't been able to
get in MPD-mode for a few weeks.

I take your point that the commit message could be more verbose, however
most commit messages I make in my day job tend to read more like a newspaper
headline (rather than have the story with the details of how and why the
change was done). The MPD git repo suggests similar usage. I am keen for my
changes to be merged, so I decided to write a little about the problem I was
attempting to solve and potential issues with my approach so that others may
comment. Maybe I should have explained what I was attempting to do / ask for
input on the list before submitting a patch?
I also accept your point about the multi-feature patch. This was a result of
adding debug / logic tweaks as I was attempting to implement / debug the
main change. Now it is one commit in my local repository, what is the best
way of splitting it into two patches? If someone can give me some pointers,
I will split into a debug/logic patch and a DLNA streaming one.

Regards,

Steve.
On 2 September 2011 18:03, Max Kellermann <m...@duempel.org> wrote:

> On 2011/08/31 01:12, Steven Blackburn <st...@beeka.org> wrote:
> > I don't know how much explaination of the changes you normally require,
> so
> > sorry if I have gone on a bit. Could somebody merge my patch / comment on
> > any changes it needs / try it with another DLNA device.
>
> Hi Steve,
>
> thanks for your code.  Looks pretty simple.
>
> What I would like to see:
>
> If you want to describe your changes, use the commit message, not a
> long email.  If a patch needs an email text for explanation, then the
> commit message is not good enough!
>
> For easier code review, separate changes should be separate patches.
> I might be convinced of small cleanup patches pretty quickly and merge
> them immediately, even if other patches need cleanups.  In your patch,
> it is unclear which hunks are needed to supporting DLNA, and which
> ones are just some generic useful additions.
>
> Some other changes (such as logging "unreachable code reached") are
> not favored by me; I might leave them out until somebody convinces me.
> (In that case, I'd prefer an assert(false) to make it very clear that
> something extremely bad has happened, that needs developer attention)
>
> Obey the existing coding style; indent with tabs. (The last hunk has
> normal spaces)
>
> > PS: I've used several version control systems in the past, but git is new
> to
> > me so this patch is a product of google.... let me know if you need it
> > another way.
>
> git has a difficult learning curve, but once you get used to it ...
>
> If you want to do more work on MPD in the future, we can give you a
> public git repository on git.musicpd.org.
>
> Max
>
------------------------------------------------------------------------------
All the data continuously generated in your IT infrastructure contains a
definitive record of customers, application performance, security
threats, fraudulent activity and more. Splunk takes this data and makes
sense of it. Business sense. IT sense. Common sense.
http://p.sf.net/sfu/splunk-d2dcopy1
_______________________________________________
Musicpd-dev-team mailing list
Musicpd-dev-team@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team

Reply via email to