Skip to content

Remove simplifyQtSignalsSlots(), update qt.cfg#4807

Merged
firewave merged 6 commits intodanmar:mainfrom
chrchr-github:chr_SimpQt
Feb 25, 2023
Merged

Remove simplifyQtSignalsSlots(), update qt.cfg#4807
firewave merged 6 commits intodanmar:mainfrom
chrchr-github:chr_SimpQt

Conversation

@chrchr-github
Copy link
Copy Markdown
Collaborator

No description provided.

@chrchr-github
Copy link
Copy Markdown
Collaborator Author

We don't seem to have a way to load a library in a test and also apply the defines from it, so I just removed the tests.
Nevertheless, I think qt.cfg does everything the simplification code did.

@firewave
Copy link
Copy Markdown
Collaborator

That would be something to looking into. I do not know what does apply the defines. I would prefer to keep the tests.

You could also write a system/Python test instead.

@chrchr-github
Copy link
Copy Markdown
Collaborator Author

You would have to use Preprocessor somehow, but I couldn't figure it out. It's not done in any other test either.
The tests basically check that we can replace a define with nothing, so meh.

@firewave
Copy link
Copy Markdown
Collaborator

The usage of Preprocessor in the tests is probably flawed and possibly needs to be reworked. That's actually not something I want to look into as well as currently nothing can even be remotely finished up for a boatload of reason...

It does a bit more and it appears it just replaces things in certain places.

@chrchr-github
Copy link
Copy Markdown
Collaborator Author

chrchr-github commented Feb 18, 2023

Those checkLibraryNoReturn warnings are annoying for signals, which may not have a visible implementation. We should probably only warn if the function is completely unknown.
See https://trac.cppcheck.net/ticket/11190 and https://trac.cppcheck.net/ticket/11523

@firewave
Copy link
Copy Markdown
Collaborator

cfg tests also work 👍

And I asked for the tests to be kept since there was much more logic in that removed function which appeared to be missing elsewhere. No idea why I didn't express that.

@firewave
Copy link
Copy Markdown
Collaborator

Please remove the checkLibraryNoReturn suppressions or mark it as a draft for now as the CI will fail when this and #4809 have landed.

@chrchr-github chrchr-github marked this pull request as draft February 18, 2023 22:53
@chrchr-github chrchr-github marked this pull request as ready for review February 24, 2023 10:34
@danmar
Copy link
Copy Markdown
Owner

danmar commented Feb 25, 2023

And I asked for the tests to be kept since there was much more logic in that removed function which appeared to be missing elsewhere.

I don't have an idea about that. But removing simplifyQt and using qt.cfg that sounds very good to me 👍

@firewave firewave merged commit a0cc35e into danmar:main Feb 25, 2023
@chrchr-github chrchr-github deleted the chr_SimpQt branch March 13, 2023 11:36
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