search

dogecoin / dogecoin

very currency
MIT License
14.4k stars 2.8k forks source link

Reduce Qt warnings #3463

Open chromatic opened 3 months ago

chromatic commented 3 months ago

This should apply cleanly atop the Qt4 ifdef-removal code. There are plenty of warnings to go, but this is a good place to park it for a draft for now.

chromatic commented 3 months ago

I think the error is because we're using something older than Qt 5.11.

patricklodder commented 3 months ago

I think the error is because we're using something older than Qt 5.11.

Yes. We'd be bringing back some macros if we want to support older Qt5 than latest. However, Qt 5.15 already has patches sitting in the download site rather than new releases, so maybe we should only keep only minimum support for qt5 and only for 5.15, like we had for Qt4, and only for custom compilation?

chromatic commented 2 months ago

I agree. Let's aim for Qt 6 support and Qt 5.15 as the oldest supported fallback unless someone commits to active maintenance for something older on a specific platform.

If we do this, should we start with a configure check for Qt >= 5.15?

patricklodder commented 2 months ago

If we do this, should we start with a configure check for Qt >= 5.15?

Not as long as depends is not updated because then we'll not have a CI at all, so that means this PR will need to remain in draft until that's done. I expect quite some changes being needed for Qt6 though, so the conflicts on this PR will also skyrocket at some point...

What can be done now re: unblocking this PR is fix anything that works with 5.7, and bump the minimum up to 5.7 in the m4 - I have anyway not seen any reports using earlier than Qt5.7 for a long time (other than the specific Qt4 test I did last round)

chromatic commented 2 months ago

I see the wisdom in that. Sounds like the way to test this is in a container which has Qt 5.7 then, and exclude anything without support there. I'll see what I can do to put that together, unless someone else reading this beats me to it.

patricklodder commented 2 months ago

depends has 5.7 so you can simply test it with a standard build w/ that