Skip to content

Conversation

@hartwork
Copy link
Contributor

@hartwork hartwork commented Jul 7, 2025

@hartwork hartwork changed the title qmake: Fix Qt version checks for Qt 4.8.0 qmake: Fix Qt version check for Qt 4.8.0 Jul 7, 2025
@hartwork hartwork force-pushed the fix-qmake-qt-version-check branch from 13fc3f8 to d33bce8 Compare July 7, 2025 10:13
@barracuda156
Copy link
Contributor

@hartwork That should work, I just kept a version which existed already, see: c0fb7c1

@tibirna
Copy link
Owner

tibirna commented Jul 7, 2025

To be pedantic, lessThan is not the same as !greaterThan (in this case, the latter excludes 4.8.0). Please indicate what does this intend to fix?

@hartwork
Copy link
Contributor Author

hartwork commented Jul 7, 2025

@tibirna the messaging and in-code comment further up both indicate that >=4.8.0 is wanted but the code is actually checking for >4.8.0 without allowing for =4.8.0. The fix gets that back in sync.

greater_equal

@barracuda156
Copy link
Contributor

@hartwork TBH, I have no idea what is the actual requirement (which is also why I didn’t bother changing that to match). But perhaps it is better to make it coherent to avoid confusion. If someone ever tries to build this with Qt 4.8.0 and that happens not to work, they could open an issue.
On the other hand, it can make sense to require 4.8.7, just to be on the safe side.

@hartwork
Copy link
Contributor Author

hartwork commented Jul 7, 2025

@barracuda156 good to know. My mission here was just to make the code do what the comments say it's supposed to do. Maybe that's a good start?

@hartwork hartwork changed the title qmake: Fix Qt version check for Qt 4.8.0 qmake: Get Qt version check in sync with its documented intention Jul 7, 2025
@tibirna tibirna merged commit eb8479a into tibirna:master Jul 7, 2025
1 check passed
@hartwork hartwork deleted the fix-qmake-qt-version-check branch October 15, 2025 21:54
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.

3 participants