[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=694287

--- Comment #45 from Richard Shaw <hobbes1069 at gmail.com> 2011-04-27 11:17:40 EDT ---
(In reply to comment #44)
> Almost there, but still needs some work:
> 
> * Please drop everything in the spec file above Name: except for the %define of
>   AGE, the rest is no longer needed

Done.


> * Please drop Patch1 and Patch2 they are not needed, they patch SConscript
>   files which are only used by scons ...

I think you mean patch 0 and patch1 since patch2 is your patch. Done.


> * Please drop "License:        MIT" from the -devel subpackage, since it
>   is the same as for the main package (I missed that before)

Done.


> * Remove %{?_smp_mflags} from your make invocation, otherwise the build fails
>   at least it does so consistently on my quad core.
>   Funny how you kept the comment from me snippet saying that the build breaks,
>   but re-added the %{?_smp_mflags} :)

When you said it didn't work I didn't think you meant failed to build, just
that it didn't do anything. It works for me on my dual core with -j4. Maybe we
should spec a max number of threads conditional?


> * You're still installing header files from the common dir, resulting in
>   unittest and performance test headers ending up under /usr/include, but
>   see the next item for a more radical suggestion for re-arranging the
>   headers.

Should I remove common in addition to the below suggestion? What about
Externals/MathMLSolver?


> * And last, one slightly larger issue (which I should have checked before).
>   I'm not really happy with putting a bunch of the .h files directly under
>   /usr/include. Ideally (IMHO) COLLADAfoo/include/* should end up as
>   /usr/include/COLLADAfoo/* for all variants of foo

Hmm. This sounds like a job for bash and my bashfoo is not that strong :)

I'm thinking some sort of for loop that either inverts foo/include to
include/foo or that strips the ./include off before copying the files.

Another option would be to create a /usr/include/openCOLLADA and then just dump
all the files and/or directories under that...

What do you think?

Richard

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.