klenin / Spawner

Cross-platform sandbox runner
4 stars 8 forks source link

CMake: Various fixes in libspawner project. #52

Closed cher-nov closed 7 years ago

cher-nov commented 7 years ago
klenin commented 7 years ago

@ViktorPegy, please review. My suggestion would be to remove sorting to avoid conflicts and merge the rest.

cher-nov commented 7 years ago

@klenin, @ViktorPegy: attached a merged version of CMakeLists.txt from this PR and this one. Line endings are LF.

Attach: CMakeLists.txt

If this PR will be merged, @ViktorPegy could perform "fetch & rebase" from and onto upstream, replace conflicted CMakeLists with this one and finish rebasing.

This is just a suggestion. If you decide to remove sorting, I will do this.

dsbogdan commented 7 years ago

Hi cmake file(GLOB ...) feature may help to eliminate manual roll out of all project filenames (a patch is rather simple).

just my 2c.

klenin commented 7 years ago

@dsbogdan: not so simple, see 9ceddd432a2355809596b199ca977926aaeca98e Maybe @rotanov will comment

rotanov commented 7 years ago

I tried to bring GLOBs back now and succeeded, so GLOBs are the way, sure. I have only slightest idea now of why I removed GLOBs. Here are some pros and cons of using GLOB:

pros:

cons:

I think the first point of cons was the deal breaker for me. And I'm basically always prefer explicit to implicit. e.g. implicit cast

klenin commented 7 years ago

@cher-nov: So please resubmit useful parts separately. Generally, please do not mix small fixes with large breakage within a single PR.