Skip to content

Comments

fix compass not working in 8.0.0#252

Merged
gr5 merged 1 commit intomasterfrom
JST/fix249
Oct 27, 2025
Merged

fix compass not working in 8.0.0#252
gr5 merged 1 commit intomasterfrom
JST/fix249

Conversation

@atsju
Copy link
Collaborator

@atsju atsju commented Oct 20, 2025

see #249. Compass was broken.

I was confident that the new style connect is wonderful and everything is checked at compile time. This is only partially true.
While the signal/slot existence is checked at compile time you can still fail at runtime because of what could be ABI difference between library and main application.

So for the moment, even if I'm not happy with this fix (not explaining/fixing the root cause) I propose to revert to old style to get the application to work.

When testing if you see any missed connect in logs, please inform me so that I revert them too.

@atsju atsju requested review from githubdoe and gr5 October 20, 2025 17:45
Copy link
Collaborator

@gr5 gr5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know the proper syntax for signals and slots. I just create them using the QT gui and they just work for me so far. Did some for my new autoinvert dialog recently.

But I approve these changes because I looked it over and you are only changing a few things and they can be tested when we get the release.

I plan to do more than normal testing on the next release.

@atsju
Copy link
Collaborator Author

atsju commented Oct 20, 2025

I need to add, the changes are not only small but I reverted to original code from before my changes in connection syntax. Copied from git history.

If you see any warning log during your extended tests, let me know.

@gr5
Copy link
Collaborator

gr5 commented Oct 27, 2025

Julien, I think we should go ahead and merge these PRs and make a new release. Or at least this one.

@atsju
Copy link
Collaborator Author

atsju commented Oct 27, 2025

@gr5 I think they can all be merged. For me everything is OK. I'm at work and will not have too much time. Feel free to merge any of the open PR yourself

@gr5 gr5 merged commit ae7b845 into master Oct 27, 2025
14 checks passed
@atsju atsju deleted the JST/fix249 branch November 10, 2025 07:10
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.

2 participants