Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reorganize directories #156

Closed
wants to merge 1 commit into from
Closed

Reorganize directories #156

wants to merge 1 commit into from

Conversation

elalish
Copy link
Owner

@elalish elalish commented Jul 18, 2022

This is one idea I had based on @fire's suggestion to reorganize our directories to make usage more clear. I want to make it more obvious what the core C++ library is with minimal dependencies, and separate all the extra stuff that's important, but optional. The idea is now we'd have two third_party folders, one for core dependencies and another for other dependencies. It also makes it more clear that meshIO is not a core part of the library, but really a support for test/debug.

I'd like to get some feedback on this before I go through and make it all build (and I might need some help with that @pca006132 馃檹 ). Does this structure look like it'll be more clear and easier to depend on?

Wow, I was hoping Github would do a better job on the diff; that's pretty much unreadable. Okay, the change is to go from:

root/
|--- docs/
|--- collider/
|--- manifold/
|--- meshIO/
|--- polygon/
|--- samples/
|--- test/
|--- third_party/
|    |--- assimp/
|    |--- glm/
|    |--- google_test/
|    |--- graphlite/
|    |--- pybind11/
|    |--- thrust/
|--- tools/
|--- utilities/

to this:

root/
|--- docs/
|--- collider/
|--- manifold/
|--- polygon/
|--- third_party/
|    |--- glm/
|    |--- graphlite/
|    |--- thrust/
|--- utilities/
|--- extras/
|    |--- meshIO/
|    |--- samples/
|    |--- test/
|    |--- tools/
|    |--- third_party/
|    |    |--- assimp/
|    |    |--- google_test/
|    |    |--- pybind11/

@pca006132
Copy link
Collaborator

pca006132 commented Jul 18, 2022

My two cents:

root/
|--- docs/
|--- examples/
|    | --- ...samples
|--- src/
|    |--- collider/
|    |--- manifold/
|    |--- polygon/
|    |--- utilities/
|--- test
|    |--- meshIO
|    |--- ...original tests, referencing examples
|--- extras
|    |--- ...tools
|--- third_party
|    |--- ...

The idea is to use a familiar structure, source files are grouped into src in root. I think we can just add a build instruction in the README to say that some third party dependencies are optional (test depends on assimp and google_test, python binding depends on pybind11).

@elalish
Copy link
Owner Author

elalish commented Jul 18, 2022

Yes, I think I like that better. Still, I think I'd like to split up third_party to make it clearer what depends on what. I think people are more likely to understand our dependency graph from directory structure than by reading our README (though it should be there too). How about this:

root/
|--- docs/
|--- bindings/
|    |--- python/
|    |    |--- third_party/
|    |    |    |--- pybind11/
|    |--- wasm/
|--- examples/
|    |--- ...samples
|--- extras/
|    |--- ...tools
|--- src/
|    |--- collider/
|    |--- manifold/
|    |--- polygon/
|    |--- utilities/
|    |--- third_party/
|    |    |--- glm/
|    |    |--- graphlite/
|    |    |--- thrust/
|--- test/
|    |--- meshIO/
|    |--- ...original tests, referencing examples
|    |--- third_party/
|    |    |--- assimp/
|    |    |--- google_test/

@pca006132
Copy link
Collaborator

LGTM

@fire
Copy link
Contributor

fire commented Jul 18, 2022

Looks good.

@elalish elalish mentioned this pull request Jul 19, 2022
@elalish
Copy link
Owner Author

elalish commented Jul 19, 2022

Thanks, closing in favor of #157.

@elalish elalish closed this Jul 19, 2022
@elalish elalish deleted the organize branch July 19, 2022 05:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants