search

patrikhuber / eos

A lightweight 3D Morphable Face Model library in modern C++
Apache License 2.0
1.89k stars 596 forks source link

Update .gitignore #233

Closed YF-Tung closed 5 years ago

YF-Tung commented 5 years ago

Ignore build/ and install/ so one can build with /eos/build$ cmake .. -DCMAKE_INSTALL_PREFIX=../install/ Therefore it's easier to be managed as a submodule.

patrikhuber commented 5 years ago

Thanks for contributing.

I'm not sure I'm in favour of merging this. In my opinion people should not use in-source builds, and the build and particularly install directory should always go outside the source directory. Having these in the .gitignore might encourage people to do in-source builds or makes them less realise that what they're doing is probably not the best practice. Also, different people may have different conventions on how to name their build and install directories.

Could you perhaps elaborate a bit more on why an in-source build is needed or more convenient when using submodules?

YF-Tung commented 5 years ago

Thanks for replying. What you said really makes sense.

Still let me share my situation here. When I include some 3rd party projects into mine, I usually put them into somewhere like 3rdparty/. There may be multiple 3rd party repos here, so it seems tidier to have a single directory for each repo. Hence in case of I don't need it anymore, I can safely delete it without thinking if I left some binaries behind elsewhere.

Nevertheless what you said is still reasonable. My situation could be solved for adding a build directory for 3rd party. I suggest you can either merge this while encouraging users to build at somewhere you intended, or ignore this PR. I'm alright with either.