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

Feature "qtwidgets" added. #294

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Atlas42
Copy link

@Atlas42 Atlas42 commented Aug 17, 2023

Link with QtWidgets and use QApplication when activated (default) Use QGuiApplication when not.

Replaces #288

I rewrote the patch in a new clean branch, keeping the master for my own projects dependency.

As #[cfg] and cpp! does not work well (if not at all together), I used a C++ predefined symbol to configure the used class.
Not as clean as I would like but I lost too much time already to figure out somthing that works.
Tell me if you have a better solution.

Link with QtWidgets and use QApplication when activated (default)
Use QGuiApplication when not.
@ratijas ratijas added C-feature-request Category: A feature request, i.e: not implemented / a PR. A-C++ Area: C++ glue and removed C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Aug 17, 2023
@ogoffart
Copy link
Member

This is a breaking change so it will require a major version update.

@Atlas42
Copy link
Author

Atlas42 commented Aug 17, 2023

major as in 1.0.0 or as 0.3.0 ?
any idea when that will happen ? not a big fan of keeping custom versions for a long time.

Any idea on implementing this without making it a breaking change ?
I thought about using a "no-qwidgets" feature instead but I find this rather dirty.

@ogoffart
Copy link
Member

0.3

Could happen soon.
@ratijas is there any other change you would like to do?

@ratijas
Copy link
Collaborator

ratijas commented Aug 17, 2023

nah, it's not like I'm actively working with anything in Rust lately. But thanks for asking!

Maybe we could consider finilizing and merging #227 as well?

@ratijas
Copy link
Collaborator

ratijas commented Aug 17, 2023

Since this PR seems to be replacing #211, how about adding a little note to README just like they did?

@ratijas
Copy link
Collaborator

ratijas commented Aug 18, 2023

is there any other change you would like to do?

Also, this

Or we can just keep this QObjectBox as deprecated and remove it when we have reason to break semver.

#178

@Atlas42
Copy link
Author

Atlas42 commented Aug 18, 2023

README entry added, my PR is almost identical to #211

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-C++ Area: C++ glue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants