-
Notifications
You must be signed in to change notification settings - Fork 90
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
Forward Qt logging to Rust log!() facade #86
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks. I'll merge soon.
@@ -11,13 +11,15 @@ keywords = ["Qt", "QML", "QMetaObject",] | |||
repository = "https://github.com/woboq/qmetaobject-rs" | |||
|
|||
[features] | |||
default = ["log"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I'm just concerned about having too many dependencies. That's also why i wouldn't put it as the default either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think debugging is essential, especially on such multi-lingua projects, and should be on by default. By having it on by default AND providing instructions for activation on the front-page example we are lowering the barrier for newcomers.
|
||
### `log` | ||
|
||
By default, Qt's logging system is not initialized, and messages from e.g. QML's `console.log` don't go anywhere. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't they go in the stderr by default?
I mean, they follow the default Qt handler which is also quite configurable with env variable: https://doc.qt.io/qt-5/debug.html#warning-and-debugging-messages / https://doc.qt.io/qt-5/qloggingcategory.html#configuring-categories
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idk, by default they just not showing up at all on my system. I haven't been able to figure out why, but now, after manual activation, it works, which is fine for me.
This is breaking change for old code.
Breaking change incoming: 3da8fe7 |
This is an implementation of proposal #85.