Skip to content

Conversation

@hjmjohnson
Copy link
Contributor

Replace with backward compatible recommended or
preprocessor conditional for old and new.

Use new paradigms that are backward compatible
when possible.

Use preprocessor conditionals where the syntax
is different between qt5 and qt6.

Replace with backward compatible recommended or
preprocessor conditional for old and new.

Use new paradigms that are backward compatible
when possible.

Use preprocessor conditionals where the syntax
is different between qt5 and qt6.
Comment on lines +105 to +109
#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
_metaTypeId = QMetaType::fromName(classname).id();
#else
_metaTypeId = QMetaType::type(classname);
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks - could you move this into PythonQtUtils.h, together with the other version specific stuff? That would avoid cluttering the code with too many #ifdefs. Same for the other places.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't understand your request. I'm unsure how to move this definition from the source code to the header file while keeping the compilation unit separate from the declaration.

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean adding a convenience function (something like metaTypeIdFromClassName) that encapsulates the #ifdefs, like the rest of the functions in that namespace that are defined inline (I guess, for performance reasons).

@jcfr
Copy link
Contributor

jcfr commented Aug 28, 2025

This pull request should be closed. It is superseded by:

@mrbean-bremen
Copy link
Contributor

Closing as suggested. Thanks @hjmjohnson!

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