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

cxx-qt-lib: add member functions #416

Merged
merged 17 commits into from
Mar 7, 2023

Conversation

ahayzen-kdab
Copy link
Collaborator

@ahayzen-kdab ahayzen-kdab commented Jan 17, 2023

Related to #55

Requires #412

Requires #419

@ahayzen-kdab ahayzen-kdab force-pushed the 55-member-functions branch 2 times, most recently from 25be770 to bc277ee Compare January 18, 2023 18:56
@ahayzen-kdab ahayzen-kdab force-pushed the 55-member-functions branch 3 times, most recently from f892c50 to 9be49c6 Compare January 24, 2023 03:38
@ahayzen-kdab ahayzen-kdab changed the title WIP: cxx-qt-lib: add member functions cxx-qt-lib: add member functions Jan 24, 2023
@ahayzen-kdab ahayzen-kdab force-pushed the 55-member-functions branch 4 times, most recently from 8d06694 to dceff60 Compare January 31, 2023 03:48
@ahayzen-kdab ahayzen-kdab force-pushed the 55-member-functions branch 4 times, most recently from 683a07e to 31c770f Compare February 1, 2023 01:42
Copy link
Collaborator

@LeonMatthesKDAB LeonMatthesKDAB left a comment

Choose a reason for hiding this comment

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

One actual issue on the QUrl::query implementaiton, the rest is mostly nitpicking or opinion-based.

You could consider using a Rust macro for all these _or_default methods...

crates/cxx-qt-lib/src/core/qdate.rs Show resolved Hide resolved
crates/cxx-qt-lib/src/core/qdate.rs Outdated Show resolved Hide resolved
crates/cxx-qt-lib/src/core/qdate.rs Outdated Show resolved Hide resolved
crates/cxx-qt-lib/src/core/qtime.rs Show resolved Hide resolved
crates/cxx-qt-lib/src/core/qtime.rs Outdated Show resolved Hide resolved
crates/cxx-qt-lib/src/core/qurl.rs Show resolved Hide resolved
crates/cxx-qt-lib/src/core/qurl.rs Outdated Show resolved Hide resolved
crates/cxx-qt-lib/src/core/qurl.rs Show resolved Hide resolved
crates/cxx-qt-lib/src/core/qurl.rs Show resolved Hide resolved
crates/cxx-qt-lib/src/core/qurl.rs Show resolved Hide resolved
Copy link
Collaborator

@LeonMatthesKDAB LeonMatthesKDAB left a comment

Choose a reason for hiding this comment

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

Again, a bit more minor stuff.
Overall, amazing work!

crates/cxx-qt-lib/src/gui/qcolor.rs Outdated Show resolved Hide resolved
crates/cxx-qt-lib/src/gui/qcolor.rs Outdated Show resolved Hide resolved
@ahayzen-kdab ahayzen-kdab force-pushed the 55-member-functions branch from 3a707c4 to 1db0d59 Compare March 7, 2023 15:39
Copy link
Collaborator

@LeonMatthesKDAB LeonMatthesKDAB left a comment

Choose a reason for hiding this comment

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

LGTM 🥳

@ahayzen-kdab ahayzen-kdab enabled auto-merge (rebase) March 7, 2023 15:52
@ahayzen-kdab ahayzen-kdab merged commit 6745f22 into KDAB:main Mar 7, 2023
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.

2 participants