-
Notifications
You must be signed in to change notification settings - Fork 126
Fix MacOS C++ tests compatibility issues #3677
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
Changes from all commits
f1a5999
3f999c4
13405a6
d822e30
58d7e18
779d584
28fcbe4
e6ed708
793110c
031ab83
bbf6240
0dcc1c5
40856ba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -120,7 +120,13 @@ pyqtgraph==0.13.7 \ | |
| --hash=sha256:64f84f1935c6996d0e09b1ee66fe478a7771e3ca6f3aaa05f00f6e068321d9e3 \ | ||
| --hash=sha256:7754edbefb6c367fa0dfb176e2d0610da3ada20aa7a5318516c74af5fb72bf7a | ||
| # via -r software/thunderscope/requirements.in | ||
| qtawesome==1.4.0 \ | ||
| --hash=sha256:783e414d1317f3e978bf67ea8e8a1b1498bad9dbd305dec814027e3b50521be6 \ | ||
| --hash=sha256:a4d689fa071c595aa6184171ce1f0f847677cb8d2db45382c43129f1d72a3d93 | ||
| # via -r software/thunderscope/requirements.in | ||
|
Comment on lines
+123
to
+126
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are these intended dep updates?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do they need to be done, seems to be something fit for another pr (or for #3542, I promise I'll finish that soon)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One of the software tests that failed was for the requirements; I just updated the requirements cause it was supposed to be done way earlier |
||
| qtpy==2.4.2 \ | ||
| --hash=sha256:5a696b1dd7a354cb330657da1d17c20c2190c72d4888ba923f8461da67aa1a1c \ | ||
| --hash=sha256:9d6ec91a587cc1495eaebd23130f7619afa5cdd34a277acb87735b4ad7c65156 | ||
| # via pyqt-toast-notification | ||
| # via | ||
| # pyqt-toast-notification | ||
| # qtawesome | ||
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.
This looks fine, but just curious:
Is there any reason that we did not use polyfills such as
https://github.com/adobe/lagrange/blob/771d85889dd052b545de1fa4a66fe4b3ff2c5e91/modules/core/src/fpe.cpp#L94-L113
Not sure if polyfills like this will work, but I think if it does, it will enable more consistency behaviours across different platforms?
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.
Oh yea I tested it and it should technically be possible since I got it working for a minimal example https://gist.github.com/nycrat/3bfd574a7b3d328e51ae1b3705de9ebc. But for some reason it doesn't work with gtest. I'll keep trying to make it work since that would be optimal
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.
Yea I can't seem to get it working, something with the gtest environment I guess?