Hi Max, Max Kellermann wrote: > I don't see what you modified. Did you add the original (unmodified) > sources, or did you only add the versions which were already modified > by you? The parser has already been modified by me.
> The "clean" way to do it would be to create one commit which adds the
> original sources, and another commit edits them and adds them to the
> build system (Makefile.am).
That'll be some work..
>> commit 0ce399a344093caa24328971b51f74039597e749
>> Author: Jochen Keil <jochen.k...@gmail.com>
>>
>> Makefile for the lexer/parser
>>
>> BSD style only for the moment.
>> It is easy to understand though if you want to compile
>> the test program or generate the lexer/parser.
>
> What's this for?
It's for the one who is interested to build the lexer/parser himself.
You don't need it though, so leave it out if you want to. I have to
admit that i don't know enough about automake/autoconf to add a build
lexer/parser command or so..
>> commit 755f16d9d0f09e11cea836e83d8ea47b002133f6
>> Author: Jochen Keil <jochen.k...@gmail.com>
>>
>> Quick and dirty test program for the cuesheet parser
>>
>> Will not be compiled by mpd make
>
> Why not? Why not make it a test program in ./test/? This should live
> in test/, not in src/cue_parser/. If you don't compile it, don't add
> it to the source tree.
I'll move it to /test.
>> commit eebbd9a8043f301c312e3e6f63654c12314b4a70
>> Author: Jochen Keil <jochen.k...@gmail.com>
>>
>> The implementation details for cue.h
>
> Who's the author? Why does it include "cd.h"? Why do we need "cd.h"
> at all? Looks like it's dealing with information for "compact discs",
> but MPD doesn't handle CDs.
The author of the original cuetools package is Svend Sorensen.
cd.h contains the methods for accessing the struct which holds the
information from the parsed cuesheet. Svend called that struct Cd, thus
cd.h/.c. I have no problem with that but if you'd like a different name
for it i can change that.. as i said i wanted to rework the api anyway.
> Don't create a separate commit for each and every source file you
> import. cd.h is nothing without cd.c.
Should i add them all at once or cd.h + cd.c, cue.h + cue.c, etc.?
> You're not the author of those files, yet the git commit reflects your
> name. I believe this is important.
I've left the copyright notice from Svend Sorensen in there but when i
recommit the original sources as you proposed above i'll commit them in
Svend's name.
> Anyway, what's the license of that source code?
GNU GENERAL PUBLIC LICENSE
Version 2, June 1991
> Each commit which adds a new source must add it to Makefile.am. Each
> commit must not fail during build. "make distcheck" should always
> work, too.
So it should be like this:
new file (e.g. cd.c) + add to Makefile.am = one commit?
>> Generating the lexer/parser using lex/yacc or flex/bison
>> isn't necessary every time for compiling.
>> That way we don't have to depend on those programs.
>
> I disagree. *Always* generate those sources. CUE support should be
> optional, so if lex/yacc are not available, don't build CUE support.
> We should never have generated files in git. Having yet another
> dependency isn't comfortable for users, but with this excuse, we could
> integrate everything into MPD. MPD used to have libmad built-in.
I'm fine with this, but someone got to help me then on how to add
flex/bison/cuesheet dependency to Makefile.am.
Maybe i'll read a bit on that too but it's going to take a while.
For commiting will it suffice to set user.name and user.email to Svend's
contact information?
Should i also add the files from cuetools i removed (and remove them
again from git)?
Regards,
Jochen
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Apps built with the Adobe(R) Flex(R) framework and Flex Builder(TM) are powering Web 2.0 with engaging, cross-platform capabilities. Quickly and easily build your RIAs with Flex Builder, the Eclipse(TM)based development software that enables intelligent coding and step-through debugging. Download the free 60 day trial. http://p.sf.net/sfu/www-adobe-com
_______________________________________________ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
