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.

Sign up for GitHub

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

Jump to bottom

Constructor trait #566

Merged
merged 6 commits into from
Jul 12, 2023
Merged

Constructor trait #566

merged 6 commits into from
Jul 12, 2023

Conversation

LeonMatthesKDAB
Copy link
Collaborator

Closes #550

@LeonMatthesKDAB LeonMatthesKDAB added ⬆️ feature New feature or request 🥳🎉 1.0 This issue is part of stabilization for 1.0 release labels May 30, 2023
@LeonMatthesKDAB LeonMatthesKDAB force-pushed the constructor-trait branch 7 times, most recently from 8c1978a to e95df03 Compare June 2, 2023 15:36
@ahayzen-kdab ahayzen-kdab mentioned this pull request Jun 5, 2023
Merged
@ahayzen-kdab
Copy link
Collaborator

@LeonMatthesKDAB let me know when this is ready, and I could port the QQuickPaintedItem example from #513 to work with this. That could then be a good test + example of how choosing a different parent class / base class arguments work.

@ahayzen-kdab
Copy link
Collaborator

Random thought, how do this work in combination with #[derive(Default)] or impl Default for T on the MyObject struct ?

Is it allowed to have a Constructor as well ? I guess yes ? it doesn't really affect things just means that the rust constructor could call T::default() or the developer could do what they want instead.

@ahayzen-kdab ahayzen-kdab mentioned this pull request Jun 22, 2023
Merged
ahayzen-kdab
ahayzen-kdab reviewed Jun 30, 2023
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@LeonMatthesKDAB
Copy link
Collaborator Author

Random thought, how do this work in combination with #[derive(Default)] or impl Default for T on the MyObject struct ?

Is it allowed to have a Constructor as well ? I guess yes ? it doesn't really affect things just means that the rust constructor could call T::default() or the developer could do what they want instead.

That's not an issue, as the Default only applies to Rust in that case and the Constructor implementation is totally independent and may even call default if one wants.

@LeonMatthesKDAB LeonMatthesKDAB force-pushed the constructor-trait branch 3 times, most recently from 1306667 to e964f88 Compare July 6, 2023 16:17
@LeonMatthesKDAB LeonMatthesKDAB marked this pull request as ready for review July 6, 2023 16:19
@ahayzen-kdab
Copy link
Collaborator

In general i think this looks good now, just a few comments left :-)

ahayzen-kdab
ahayzen-kdab approved these changes Jul 12, 2023
View reviewed changes
Copy link
Collaborator

@ahayzen-kdab ahayzen-kdab left a comment

Choose a reason for hiding this comment

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

LGTM, lets go with this :-)

@ahayzen-kdab ahayzen-kdab enabled auto-merge (rebase) July 12, 2023 15:21
@ahayzen-kdab ahayzen-kdab merged commit 8e41afd into KDAB:main Jul 12, 2023
@LeonMatthesKDAB LeonMatthesKDAB mentioned this pull request Jul 15, 2023
Merged
@LeonMatthesKDAB LeonMatthesKDAB mentioned this pull request Jul 28, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Be-ing Be-ing Be-ing left review comments

@ahayzen-kdab ahayzen-kdab ahayzen-kdab approved these changes

Assignees
No one assigned
Labels
⬆️ feature New feature or request 🥳🎉 1.0 This issue is part of stabilization for 1.0 release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support implementing a custom constructor from Rust
3 participants
@LeonMatthesKDAB @ahayzen-kdab @Be-ing