-
Notifications
You must be signed in to change notification settings - Fork 1.6k
extracted hard-coded platform definitions to actual platform files #4325
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
base: main
Are you sure you want to change the base?
Conversation
0ce9cf1 to
94079c3
Compare
test/testcmdlineparser.cpp
Outdated
| TEST_CASE(stdcpp11); | ||
| TEST_CASE(stdunknown); | ||
| TEST_CASE(platform); | ||
| TEST_CASE(platform2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you name the test function according to the platform file tested? e.g. platform_win64 etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those will probably go away soon anyways because of the incremental nature...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
|
This breaks Since I just wanted to get my backlog'd changes into the tree and those are incomplete this PR will unfortunately sit a while. |
|
I removed the deprecation stuff as it is part of a separate PR. |
|
The tests are affected by no platform data being set on Windows. The platform file loading is part of the command-line parsing and is not executed before the tests. When the windows platform non-builtin causes no data to be set. Before we can merge this we need to remove those first. Fortunately there are already scheduled to be removed in the next dev cycle. |
51e5d3c to
afe9af1
Compare
|
For some reason the validation is not able to read But I think we should not have this. The built-in configurations currently default to the signedness of the binary which I think is wrong. Same as we were defaulting to a Windows platform in Windows binaries. Those should possibly be We also should probably have a command-line option similar to This is something I have to think about and I am gonna finish #5414 first as this also plays into it. |
428c2d3 to
15ec9cd
Compare
|
Having |
f3f5234 to
0e20094
Compare
b14a38e to
d80b6c3
Compare
…n32A`, `win32W` and `win64` to actual platform files
…tforms `unix32-unsigned`, `unix64-unsigned` and `unspecified` [skip ci]
No description provided.