Ok that looks good, but I think it needs splitting up a bit.
For starters, your patch seems to remove quite a lot of code that isn't in the file to begin with?!? Is there another couple of patches on your branch that you need to submit as well?
If you have time, here's what I'd like:
* Please do add a header to CMakeLists.txt, and if you could add my details to it as well, I would appreciate it.
* set(VERSION 0 1 0), would be very helpful. Is there a way we can hook that up to the About box?
* Adding global sections would be helpful, but I'd prefer more concise captions. "Global Defines" and "Global Include Directories" is fine. "Back to our pulseview binaries
" is a bit ugly when you could just put "Binaries" or something like that. Also minor point: Is there a way you could delimit sections which is less heavy? That's an awful lot of "#" characters. Even using "-" would be lighter on they eye, and/or just one row, would look a lot less heavy.
* Shortening the WIN32 stuff down: Yes! Absolutely!
If you can do each of those bullet points as a single patch, that would be ideal.
I don't know if anyone else has any comments, but thanks for contributing. It's really nice to have people joining in on this!
On 11 October 2012 at 23:41 "Alex G." <email@example.com> wrote:
> Minor CMakeLists.txt cleanup
> First of all, specify the build directory as an include directory.
> This enables us to build out of source, for example:
> mkdir build
> cd build
> cmake ..
> Remove all the if(WIN32) clauses, and use an option to specify wheter we
> want to link to the static versions of libsigrok*. That's basically what
> the WIN32 clauses were doing anyway. This makes the "what is going on"
> more readeable.
> On WIN32, automatically set this option to link to the static libraries.
> Add a copyright header to CMakeLists.txt
> Signed-off-by: Alexandru Gagniuc <firstname.lastname@example.org>
> Don't let slow site performance ruin your business. Deploy New Relic APM
> Deploy New Relic app performance management and know exactly
> what is happening inside your Ruby, Python, PHP, Java, and .NET app
> Try New Relic at no cost today and get our sweet Data Nerd shirt too!
> sigrok-devel mailing list