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

[RDY] CMake Cleanup #1325

Merged
merged 8 commits into from Jan 12, 2018

Conversation

Projects
None yet
3 participants
@TheCycoONE
Member

TheCycoONE commented Jan 10, 2018

Style consistency and dead code removal for the CMake scripts.

@DavidFair

Some additional tidying opportunities

Show outdated Hide outdated CMakeLists.txt Outdated
Show outdated Hide outdated CorsixTH/CMakeLists.txt Outdated
FILES_MATCHING REGEX ".*\\.(tab|pal|dat|png)$"
PATTERN "*.svn" EXCLUDE)
FILES_MATCHING REGEX ".*\\.(tab|pal|dat|png)$"
PATTERN "*.svn" EXCLUDE

This comment has been minimized.

@DavidFair

DavidFair Jan 10, 2018

Contributor

Do we need to explicitly exclude SVN files or is this a holdover from before the project used git?

@DavidFair

DavidFair Jan 10, 2018

Contributor

Do we need to explicitly exclude SVN files or is this a holdover from before the project used git?

This comment has been minimized.

@TheCycoONE

TheCycoONE Jan 10, 2018

Member

Hold over, I can drop these.

@TheCycoONE

TheCycoONE Jan 10, 2018

Member

Hold over, I can drop these.

Show outdated Hide outdated CMake/FindLua.cmake Outdated
Show outdated Hide outdated CMake/FindLua.cmake Outdated
@TheCycoONE

This comment has been minimized.

Show comment
Hide comment
@TheCycoONE

TheCycoONE Jan 10, 2018

Member

Actually we can drop FindLua.cmake entirely because of the CMake version bump recently. It's included upstream.
You're right about the msinttypes too; and there's some other convoluted and dead code I haven't addressed yet which could belong in this PR.

Member

TheCycoONE commented Jan 10, 2018

Actually we can drop FindLua.cmake entirely because of the CMake version bump recently. It's included upstream.
You're right about the msinttypes too; and there's some other convoluted and dead code I haven't addressed yet which could belong in this PR.

@TheCycoONE TheCycoONE changed the title from [RDY] CMake cleanup to [WIP] CMake cleanup Jan 10, 2018

@TheCycoONE TheCycoONE changed the title from [WIP] CMake cleanup to [RDY] CMake cleanup Jan 11, 2018

@TheCycoONE TheCycoONE changed the title from [RDY] CMake cleanup to [WIP] CMake Cleanup Jan 11, 2018

@TheCycoONE

This comment has been minimized.

Show comment
Hide comment
@TheCycoONE

TheCycoONE Jan 11, 2018

Member

I need to update the Deps repository first because FindLua.cmake considers A/include/lua to be higher president than A/include/

Member

TheCycoONE commented Jan 11, 2018

I need to update the Deps repository first because FindLua.cmake considers A/include/lua to be higher president than A/include/

@TheCycoONE

This comment has been minimized.

Show comment
Hide comment
@TheCycoONE

TheCycoONE Jan 11, 2018

Member

Even after updating Deps we are still getting the wrong lua include. I think our FindLua.cmake was better (or at least incompatible with the upstream one), so I will restore it and fix it's issues.

Member

TheCycoONE commented Jan 11, 2018

Even after updating Deps we are still getting the wrong lua include. I think our FindLua.cmake was better (or at least incompatible with the upstream one), so I will restore it and fix it's issues.

@TheCycoONE TheCycoONE added this to the 0.70 milestone Jan 11, 2018

@TheCycoONE TheCycoONE changed the title from [WIP] CMake Cleanup to [RDY] CMake Cleanup Jan 11, 2018

@TheCycoONE TheCycoONE referenced this pull request Jan 11, 2018

Merged

[RDY] 43 world counter #1327

@@ -11,133 +11,132 @@
# - WITH_LIBAV : Whether to use LibAV (as opposed to FFMEPG) when building movies
# - WITH_VLD : Build with Visual Leak Detector (requires Visual Studio)
CMAKE_MINIMUM_REQUIRED(VERSION 3.2)
cmake_minimum_required(VERSION 3.2)

This comment has been minimized.

@DavidFair

DavidFair Jan 11, 2018

Contributor

Do we need to bump the CMake version to 3.7 for FindLua?

@DavidFair

DavidFair Jan 11, 2018

Contributor

Do we need to bump the CMake version to 3.7 for FindLua?

@DavidFair

Tested with Ubuntu and MSVC (with VCPKG) successfully on a clean build folder.

@Alberth289346 Alberth289346 merged commit 3edc6cf into CorsixTH:master Jan 12, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment