-
Notifications
You must be signed in to change notification settings - Fork 616
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Initial rename of OpenEXR and IlmBase directories and seperation of Test #796
Conversation
Is there a reason to move IlmImf into src in a following PR, instead of this one? Also, since hundreds of files have moved, would you mind posting a comment showing the output of a tool like tree to show what the directory structure looks like after the reorganization, please? |
You can just browse the new layout here, of course:
https://github.com/oxt3479/openexr/tree/RC-3
…On Thu, Jul 23, 2020 at 4:15 PM Nick Porcino ***@***.***> wrote:
Is there a reason to move IlmImf into src in a following PR, instead of
this one?
Also, since hundreds of files have moved, would mind posting a comment
using a tool like *tree* to show what the directory structure looks like
after the reorganization, please?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#796 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFC3DGPGZVUNFC4E4HFLVADR5C77HANCNFSM4PGBFI5Q>
.
--
Cary Phillips | R&D Supervisor | ILM | San Francisco
|
I think that IlmImf (the source to the main openexr-reading/writing library) should be in src, not tools. In fact, I would think the usual case would be to put all source in src (one subdirectory per library or command line tool), and "tools" (at the top level) is likely to be interpreted as scripts and tools that are used part of the build, not source for things that will be part of the installed software. |
OpenColorIO has all command-line utilities in subfolders of |
We'd talked about top level src, test, and tools, but how about this, with
everything under "src", and "bin" instead of "tools", although I'd be fine
with "apps", too:
src
├── bin
│ ├── exr2aces
│ ├── exrbuild
│ ├── exrenvmap
│ ├── exrheader
│ ├── exrmakepreview
│ ├── exrmaketiled
│ ├── exrmultipart
│ ├── exrmultiview
│ └── exrstdattr
├── lib
│ ├── Iex
│ ├── IexMath
│ ├── IlmImf
│ ├── IlmImfExamples
│ ├── IlmImfUtil
│ └── IlmThread
└── test
├── IlmImfFuzzTest
├── IlmImfTest
└── IlmImfUtilTest
…On Thu, Jul 23, 2020 at 4:35 PM Larry Gritz ***@***.***> wrote:
I think that IlmImf (the source to the main openexr-reading/writing
library) should be in src, not tools.
In fact, I would think the usual case would be to put all source in src
(one subdirectory per library or command line tool), and "tools" (at the
top level) is likely to be interpreted as scripts and tools that are used
part of the build, not source for things that will be part of the installed
software.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#796 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFC3DGJBLPMLS4LDNBWQNULR5DCM3ANCNFSM4PGBFI5Q>
.
--
Cary Phillips | R&D Supervisor | ILM | San Francisco
|
I like Cary's layout a lot. |
Will everybody throw vegetables or heavy objects at my head if I wonder out loud if we should ever switch IlmImf to OpenEXR? |
Now would be a good time to rename IlmImf, though it does provide a namespace and headers prefixed with 'Imf' not 'OpenEXR' Pedantically, IlmImfExamples is an executable binary rather than a library, though it could also be considered a test, as it's partly there to confirm the examples in the documentation actually compile |
If we're renaming and modernizing things, the "Ilm" prefix is
pretty antiquated, the only reason to retain it is historical. The "Imf"
prefix is unintuitive, although that appears in header filenames and I
don't think we should rename header files, way too much hassle. It appears
in namespace names, too, although typically via a preprocessor symbol. The
"Ilm" prefix only appears in directory names, and library names,
libIlmImf-2.5.so. But, we're changing dependencies, so everybody using the
library is going to have to make changes to their application build setup.
Would it be weird for the header files to be named like ImfHeader.h but the
library to be called libOpenEXR-3.0.so?
And I think IlmImfExamples should go in the bin directory, I don't think it
qualifies as a "test".
…On Thu, Jul 23, 2020 at 5:07 PM Larry Gritz ***@***.***> wrote:
Will everybody throw vegetables or heavy objects at my head if I wonder
out loud if we should ever switch lmImf to OpenEXR?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#796 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFC3DGKGWZVV6KL46Q6NPD3R5DGFDANCNFSM4PGBFI5Q>
.
--
Cary Phillips | R&D Supervisor | ILM | San Francisco
|
I do really really wish that the library was libOpenEXR.so and not libIlmImf.so. I think that's very counter-intuitive, hard to say out loud, hard to parse in writing because in many fonts you can't tell if it's LLm IIm (eye eye em -- see, you can't tell!) ILm or LIm, and I feel like I've had the awkward conversation 1000 times when I'm talking about libIlmImf (I use its correct name because I'm pedantic) and then have to parenthetically explain, in case someone in my audience doesn't already know what that is, that I'm talking about the library for OpenEXR, but it's not called that. Strangely, I do not feel strongly about the "Imf" in the names of headers themselves. Maybe because
is not at all ambiguous, it's quite clear in context that the header is part of OpenEXR. So I feel no urge whatsoever to change the names of the headers. The namespace issue is trivial to have your cake and eat it, too:
|
Based on the feedback I think I will begin with restructuring using a baseline of the outline posted by Cary. At the same time I'll see what can be done about renaming IlmImf, most likely just changing the namespace and directory names but leaving the same headers. |
I don't think the namespace needs to change, it doesn't contain "Ilm", and
within the OpenEXR libraries all references to it are symbolic anyway, via
the preprocessor macro.
…On Fri, Jul 24, 2020 at 9:24 AM owdt ***@***.***> wrote:
Based on the feedback I think I will begin with restructuring using a
baseline of the outline posted by Cary. At the same time I'll see what can
be done about renaming IlmImf, most likely just changing the namespace and
directory names but leaving the same headers.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#796 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFC3DGOHNP7KBP27IK5LZY3R5GYTTANCNFSM4PGBFI5Q>
.
--
Cary Phillips | R&D Supervisor | ILM | San Francisco
|
Signed-off-by: Owen Thompson <oxt3479@rit.edu>
I agree with Peter that the examples shouldn't be in lib, so perhaps src/examples instead of src/lib/IlmImfExamples |
If we're going to rename the library, is there a good reason to keep all the sub-folders in separate libraries any more? Meaning maybe we just have libOpenEXR.so as a singular .so instead libIlmImf.so, libIlmThread.so, etc.? |
… IlmBase and OpenEXR into one project. Signed-off-by: Owen Thompson <oxt3479@rit.edu>
Signed-off-by: Owen Thompson <oxt3479@rit.edu>
Signed-off-by: Owen Thompson <oxt3479@rit.edu>
Signed-off-by: Owen Thompson <oxt3479@rit.edu>
src ├── bin │ ├── exr2aces │ ├── exrbuild │ ├── exrenvmap │ ├── exrheader │ ├── exrmakepreview │ ├── exrmaketiled │ ├── exrmultipart │ ├── exrmultiview │ └── exrstdattr ├── lib │ ├── Iex │ ├── IexMath │ ├── OpenEXR │ ├── OpenEXRUtil │ └── OpenEXRThread └── test
├── OpenEXRExamples Additionally the IlmBase Library (containing only Iex and IlmThread after the removal of Imath) has been removed and Iex and IlmThread have been moved into the OpenEXR library. |
Signed-off-by: Owen Thompson <oxt3479@rit.edu>
…or Imath before including subdirs Signed-off-by: Owen Thompson <oxt3479@rit.edu>
Looking nicer and nicer. To Cary's point, I also think it's cleaner to move examples out of lib, since examples aren't libraries. It's pretty normal in a repo to see examples as a top level sibling. |
Signed-off-by: Owen Thompson <oxt3479@rit.edu>
Looks good. A few small requests before merging:
|
Signed-off-by: Owen Thompson <oxt3479@rit.edu>
… to docs/ Signed-off-by: Owen Thompson <oxt3479@rit.edu>
Signed-off-by: Owen Thompson <oxt3479@rit.edu>
…m CMake Signed-off-by: Owen Thompson <oxt3479@rit.edu>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good as an initial step, further refinements can come later.
This PR effectively:
PyIlmBase/ has not been touched as it could potentially be removed altogether, the only thing it contains now are pyIex and pyIexTest. This PR by default will not build PyIlmBase, but it remains as source.
After this PR:
This is a big change so if there are objections that is understandable. I want to get feedback before getting too far into things.