Skip to content
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

Adjust to KF6 KNotification API change #511

Merged
merged 1 commit into from
Sep 29, 2023

Conversation

kbroulik
Copy link
Contributor

setWidget has been removed in favor of setWindow.

setWidget has been removed in favor of setWindow.
@GitMensch
Copy link
Contributor

This possibly relates to the removal of the original function https://invent.kde.org/frameworks/knotifications/-/merge_requests/123 - but QT6 builds error with

mainwindow.cpp:158:23: error: ‘class KNotification’ has no member named ‘setWindow‘

because that old PR https://invent.kde.org/frameworks/knotifications/-/merge_requests/16 was only merged recently (1 month ago) so this change is reasonable but the macro used is wrong (not sure if there's actually a usable one yet).

@milianw
Copy link
Member

milianw commented Sep 28, 2023

@kbroulik ping? can you update the MR please?

@kbroulik
Copy link
Contributor Author

Update to do what?

@milianw
Copy link
Member

milianw commented Sep 28, 2023

To fix the version check, see the failing test on our Qt6 CI which doesn't yet have this new API apparently:

https://github.com/KDAB/hotspot/actions/runs/6289218537/job/17096541668?pr=511

@kbroulik
Copy link
Contributor Author

kbroulik commented Sep 28, 2023

Then I suppose your neon is outdated. KF6 has no “version” yet, it’s all git master in-flux. I have been using hotspot like this for the past few days on my neon under Plasma 6 just fine.

@milianw
Copy link
Member

milianw commented Sep 28, 2023

🤦‍♂️ thanks for clearing that up, now it's obvious that the error is on our side. I'll see when I get around to update the image and then merge this. Doing so now would break our CI - sorry. I should find the time for that on the weekend I hope.

@milianw milianw merged commit 1a557bd into KDAB:master Sep 29, 2023
12 checks passed
@milianw
Copy link
Member

milianw commented Sep 29, 2023

thanks @kbroulik - I updated our docker image for neon, and it works as expected now.

Cheers!

@kbroulik kbroulik deleted the kbroulik/knotification-api-change branch October 1, 2023 12:06
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