GeometryCollective / boundary-first-flattening

MIT License
768 stars 96 forks source link

Move headers to include/bff #32

Closed aschier closed 1 year ago

aschier commented 5 years ago

What do you think about moving your headers into include/bff folders in the projects?

I am currently trying to couple BFF with some of our mesh code and there are for example a lot of different mesh.h to consider, so including bff/mesh.h would be better to avoid conflicts between the different projects.

I ask here before doing it myself, because moving them in my own fork will make it hard to merge any changes later on when you do not like to move the headers in your project.

rohan-sawhney commented 5 years ago

There is a namespace bff for most files in the project, that should help avoid conflicts right? Or is it just the name of the file that's somehow conflicting with the build process?

aschier commented 5 years ago

Yes, the namespace helps a lot inside the code, but when I try to include mesh.h or other common names, the filename is not unique and the file which comes first in the include path is used. Grouping the own headers in a directory helps to avoid such conflicts. Especially names like Common.h easily conflict with others, so it is nice when libraries use a subfolder which clearly separates the file from other headers.

I am currently working on extending the CMake files with exported targets and install targets for the library, so it can easily be linked with other projects. I will push the changes when I verified that it can be linked in our projects. Note: I am on holiday from now to the mid march, and I will continue working on this afterwards.

aschier commented 5 years ago

I currently moved Mesh to BFFMesh (https://github.com/aschier/boundary-first-flattening/commit/1a95e862834d41b080c70d00f972cc5de35ffe4e) to avoid the problems with our header files. Our future additions to the code will depend on the new name.

I would really appreciate when the headers would be moved into a bff subfolder to avoid any future conflicts with the filenames. When you agree to merge it, I can implement it and send a pull request for the change.