Skip to content

build: remove unnecessary property sets (NFC) #24827

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

Merged
merged 1 commit into from
May 18, 2019

Conversation

compnerd
Copy link
Member

These properties are extraneous. They are never queried, and the
set(...CACHE) which precedes them will ensure that the value is
cached. This should be functionally equivalent.

Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.

Resolves SR-NNNN.

@compnerd
Copy link
Member Author

CC: @gottesmm @Rostepher

@jrose-apple
Copy link
Contributor

Doesn't this provide the set of expected values?

@Rostepher
Copy link
Contributor

Turns out the STRINGS property is only for the CMake GUI, but CMake does not enforce the options at all.

https://cmake.org/cmake/help/latest/prop_cache/STRINGS.html

@Rostepher
Copy link
Contributor

@compnerd We should remove the one in cmake/modules/StandaloneOverlay.cmake as well then.

@compnerd compnerd force-pushed the selling-properties branch from 4e6cfbf to 44254f9 Compare May 16, 2019 17:56
@Rostepher
Copy link
Contributor

@swift-ci please smoke test

@jrose-apple
Copy link
Contributor

Aw, the CMake GUI is nice sometimes, though.

@compnerd
Copy link
Member Author

@swift-ci please smoke test macOS platform

@compnerd
Copy link
Member Author

@jrose-apple - yeah, it is nice sometimes, but the description already lists the expected values

These properties are extraneous.  They are never queried, and the
`set(...CACHE)` which precedes them will ensure that the value is
cached.  This should be functionally equivalent.
@compnerd compnerd force-pushed the selling-properties branch from 44254f9 to 4d5470f Compare May 17, 2019 21:33
@compnerd
Copy link
Member Author

@swift-ci please smoke test and merge

@swift-ci swift-ci merged commit 460709f into swiftlang:master May 18, 2019
@compnerd compnerd deleted the selling-properties branch May 18, 2019 18:02
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.

4 participants