-
-
Notifications
You must be signed in to change notification settings - Fork 968
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
Lot of small codefixes related to configure #7363
Merged
Merged
+172
−165
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
df30f0a
to
7850330
Compare
michicc
reviewed
Mar 10, 2019
7850330
to
68d7b5f
Compare
It is the only library we use that calls itself with 'lib' in the name. This might be confusing, but with the arrival of cmake a lot of these things are automated. And detection will find 'liblzma', not 'lzma', like with 'lzo', 'zlib', ..
68d7b5f
to
3c1ef31
Compare
I'm not really a fan of |
… files) By naming it in a different way, things get a bit confusing. Especially if we are switching to CMake, which autodetects these things, we need to use the name the authors of ICU gave it; not our interpertation of that name.
… files) By naming it in a different way, things get a bit confusing. Especially if we are switching to CMake, which autodetects these things, we need to use the name the authors of ICU gave it; not our interpertation of that name.
config.lib happens to set GLOBAL_DATA_DIR in case it is not DOS and not OS2, but this kind of deduction is annoying to maintain. It is better to just check if the define you want to use is set, and leave it to config.lib to set it or not depending on the OS.
…viour If it was compiled with MingW, both / and \ were accepted as path separator. On MSVC, only \ was. This is an unexpected difference between binaries for the same platform. Remove this discrepancy by accepting both / and \ on all platforms.
This is more in trend with other files, where if the driver is not selected, we don't even attempt to compile it.
3c1ef31
to
3bdd904
Compare
Convention dictates uppercase, but it looked silly. Either way, fixed. |
PeterN
approved these changes
Mar 10, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
During the development of cmake, a few small things were noticed and fixed. This PR already brings that to master, to both ease up the diff the cmake branch is creating, as to reduce conflicts over time.
Please let me know if you want any of these taken out in a separate PR to talk about it in more detail; I felt most are so innocent, they are fine in a single PR.
Many individual commits have a commit body explaining their existence.