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

Use the polling crate in xclock_utc example #812

Merged
merged 1 commit into from
Apr 9, 2023

Conversation

notgull
Copy link
Collaborator

@notgull notgull commented Apr 8, 2023

Using an established ecosystem crate for polling is probably better than using system calls.

Copy link
Owner

@psychon psychon left a comment

Choose a reason for hiding this comment

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

Thanks. This seems convincing. "Going epoll" might be a bit too much for this, but getting rid of #[cfg] is definitely nice.

While looking at the polling crate, I came across their use of the log (and an issue to switch to tracing). What do you think: Would it make sense to add one of these as an (optional) dependency to x11rb?

x11rb/Cargo.toml Outdated Show resolved Hide resolved
@psychon
Copy link
Owner

psychon commented Apr 9, 2023

For future me: There was a successful AppVeyor build of the current state of things.

@notgull notgull force-pushed the notgull/polling-in-example branch from 4462006 to 7050a5b Compare April 9, 2023 16:25
@mergify mergify bot merged commit 397d582 into master Apr 9, 2023
20 checks passed
@mergify mergify bot deleted the notgull/polling-in-example branch April 9, 2023 17:15
@notgull
Copy link
Collaborator Author

notgull commented Apr 9, 2023

While looking at the polling crate, I came across their use of the log (and an issue to switch to tracing). What do you think: Would it make sense to add one of these as an (optional) dependency to x11rb?

The only reason we aren't using tracing is because tracing has an MSRV of 1.56 and the MSRV for polling is currently limited to the current Debian Stable (1.48, hopefully being bumped soon, see smol-rs/smol#244). I would support adding tracing logging to this crate.

@psychon
Copy link
Owner

psychon commented Apr 9, 2023

Okay, thanks. I'm not quite sure about log vs tracing, but they seem interoperable in both directions...

I would support adding tracing logging to this crate.

Also if I were to make it an optional dependency and hack together an abstraction to make that possible? 🙈

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.

None yet

2 participants