[opensource-dev] Review Request: STORM-1320 Create a 3p-libndofdev-linux repo based on version 0.3 of Jan Ciger's linux libndofdev.
Boroondas Gupte
sllists at boroon.dasgupta.ch
Fri Jun 17 09:42:55 PDT 2011
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/340/#review763
-----------------------------------------------------------
Ship it!
My remaining comments are mere bike-shedding, thus giving "Ship it!".
libndofdev/CMakeLists.txt
<http://codereview.secondlife.com/r/340/#comment727>
Thanks for making this a bit future-proofer. :-)
ADD_DEFINITION actually adds definitions, keeping previously added ones, doesn't it?
So we could take the '-pipe' out of the conditional code:
if (WORD_SIZE EQUAL 32)
ADD_DEFINITIONS("-m32")
elseif (WORD_SIZE EQUAL 64)
ADD_DEFINITIONS("-m64")
endif (WORD_SIZE EQUAL 32)
ADD_DEFINITIONS("-pipe")
Dunno if that's better though ... if in doubt, keep your version.
(I also thought about replacing the whole block by
ADD_DEFINITIONS("-m${WORD_SIZE} -pipe")
but I guess an if-elseif is clearer.)
libndofdev/CMakeLists.txt
<http://codereview.secondlife.com/r/340/#comment726>
Good message texts! Though, end the second sentence with a period rather than arbitrary whitespace.
- Boroondas
On June 17, 2011, 8:32 a.m., Log Linden wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/340/
> -----------------------------------------------------------
>
> (Updated June 17, 2011, 8:32 a.m.)
>
>
> Review request for Viewer, Oz Linden, Boroondas Gupte, and Altair Memo.
>
>
> Summary
> -------
>
> Checked in version 0.3 of Jan Ciger's libndofdev drop-in replacement for linux.
> * Added cmake build configuration.
> * Added autobuild package configuration.
> * Created libndofdev.txt license file from ndofdev.c file header.
> * Added README to explain that this is only for use in the linux viewer.
>
> BUGFIXES:
> * OPEN-21 STORM-312 This version of libndofdev supports kernel versions >= 2.6.33.
>
> When reviewing, please provide extra scrutiny to autobuild.xml and CMakeLists.txt, since those are the files I actually edited.
>
>
> This addresses bugs OPEN-21, STORM-1320 and STORM-312.
> http://jira.secondlife.com/browse/OPEN-21
> http://jira.secondlife.com/browse/STORM-1320
> http://jira.secondlife.com/browse/STORM-312
>
>
> Diffs
> -----
>
> BuildParams PRE-CREATION
> autobuild.xml PRE-CREATION
> libndofdev/CHANGELOG PRE-CREATION
> libndofdev/CMakeLists.txt PRE-CREATION
> libndofdev/LICENSES/libndofdev.txt PRE-CREATION
> libndofdev/README PRE-CREATION
> libndofdev/include/ndofdev_external.h PRE-CREATION
> libndofdev/ndofdev.c PRE-CREATION
>
> Diff: http://codereview.secondlife.com/r/340/diff
>
>
> Testing
> -------
>
> This built successfully on TeamCity and the packaged library worked correctly when I extracted it into the packages directory of the viewer build tree ( build-linux-i686/packages ). My spacenavigator, which hasn't worked in six months, started working with the new build.
>
>
> Thanks,
>
> Log
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.secondlife.com/pipermail/opensource-dev/attachments/20110617/e866cae0/attachment-0001.htm
More information about the opensource-dev
mailing list