-
Notifications
You must be signed in to change notification settings - Fork 89
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
Delete unused QObjectBox, update kefia example #178
base: master
Are you sure you want to change the base?
Conversation
Goodbye, you have to go. We've never been close friends anyway. Kefia example got a fresh look, although still using Quick Controls 1.x. I unironically had to find ancient ruins of Yaourt repo, build and install it, just to check the format of its -Qe output, because fields were wrongly named and that got me confused. Expac replacement is changes behavior to list all packages from all repos instead of only locally installed ones, but that's something not trivial to workaround, and its better than having forever-obsolete and forgotten dependency. [ChangeLog][Breaking Change] QObjectBox was removed. Fixes woboq#170
Now just need to figure out how all these things supposed to work together, and revise object pinning. trait QObject {}
trait QGadget {}
trait QEnum {}
struct QPointer;
struct QObjectRefMut;
struct QObjectPinned; |
That's great! However, removing QObjectBox would break semver, and i'd like to avoid doing that unless we have a good reason. |
According to Rust view of semver, 0.x minor versions don't have to be compatible, so 0.3 would be a breaking change anyway. |
By the way, kefia would be better using Rust wrappers around libalpm directly. https://crates.io/crates/alpm Maybe we should fork it completely: git-subtree copy into examples directory. That would ensure better support and up-to-date refactoring in the long run. Otherwise, I feel like I'm the only person who had patience to apply those patches. |
Yes, but I would hope that the next version would be 0.2.x
Feel free to do that. But then i guess it'd be another repository. The point of these patch stuff was just a proof of concept to show that it is possible. But I don't want to maintain that. |
I mean, not maintaining it like a useful piece of software with features and stuff. (Who needs pacman frontend anyway? Especially when KDE Discover is already doing good.) But rather just keeping it up-to-date with current code base of this framework, so that a proof of concept still holds water. It could be a separate repository, for sure. But then we'll have to link it in some new dedicated examples/showcases section somewhere in README — which is not a bad thing btw, just non-existent at the moment.
There is an option to create a 0.2 branch at the current commit, and cherry-pick critical hot fixes there. There is a re-export trick which would keep 0.2 compatible with types from 0.3 — the one they used with libc bindings. And anyway re-implementing object pinning will be a breaking change someday. |
Or we can just keep this QObjectBox as deprecated and remove it when we have reason to break semver. |
Let's stall this until pinning is revised then. It will sure bring enough reasons to break semver anyway. Otherwise, I just don't see the point — in this particular case — to keep it around for compatibility which is unlikely being used by any application code. There is no rush whatsoever. |
If QObjectBox is being removed it would be nice to get an example of how to use std::Pin instead. My use-case is setting a global property that points to QObject that I want to access from qml: let dispatcher = QObjectBox::new(Dispatcher::default());
engine.set_object_property("dispatcher".into(), dispatcher.pinned()); |
I think, patch to kefia number 5 answers your request. That was the only usage of |
I see, thanks. Honestly I think it would be better to keep this than delete it without a safe alternative in the API. The fact that it's only used in one of the examples is not necessarily representative of its usefulness overall. If it's removed I will probably copy the old QObjectBox into my code. |
FYI, I have started some work that also gets rid of QObjectPinned in favour of First commits on that branch are a merge of master and the work in this PR. |
Goodbye, you have to go. We've never been close friends anyway.
Kefia example got a fresh look, although still using Quick Controls 1.x.
I unironically had to find ancient ruins of Yaourt repo, build and
install it, just to check the format of its -Qe output, because fields
were wrongly named and that got me confused. Expac replacement is
changes behavior to list all packages from all repos instead of only
locally installed ones, but that's something not trivial to workaround,
and its better than having forever-obsolete and forgotten dependency.
[ChangeLog][Breaking Change] QObjectBox was removed.
Fixes #170
Update: almost forgot the screenshot! Dark theme looks bad with Controls 1.x, so switched to the light for a second.