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

Have EventLoopClosed contain the original event #1294

Merged
merged 2 commits into from
Dec 7, 2019
Merged

Have EventLoopClosed contain the original event #1294

merged 2 commits into from
Dec 7, 2019

Conversation

fogti
Copy link
Contributor

@fogti fogti commented Nov 30, 2019 •
edited
Loading

  • Tested on all platforms changed
    (via CI, shouldn't change behavoir of user code except involved types when using EventLoopProxy, because the return value of EventLoopProxy::send_event changed)
  • Compilation warnings were addressed
  • cargo fmt has been run on this branch
  • cargo doc builds successfully
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
    (appropriate example to modify would be examples/custom_events.rs, but I'm unsure which modification would be appropriate.)
  • Updated feature matrix, if new features were added or implemented
    (no appropriate section found, where this should be added)

Fixes #1292.

@fogti fogti marked this pull request as ready for review December 1, 2019 20:10
Osspial
Osspial reviewed Dec 2, 2019
View reviewed changes
Copy link
Contributor

@Osspial Osspial left a comment

Choose a reason for hiding this comment

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

Hi, and congratulations on your first PR!

I've attached a few review comments, but those are largely nitpicks. Overall, this looks good.

@Osspial Osspial changed the title Try to fix issue #1292 Have EventLoopClosed contain the original event Dec 2, 2019
@fogti fogti requested a review from Osspial December 4, 2019 14:27
Osspial
Osspial approved these changes Dec 4, 2019
View reviewed changes
Copy link
Contributor

@Osspial Osspial left a comment

Choose a reason for hiding this comment

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

This looks good. Thanks for the PR!

@fogti fogti requested a review from murarth December 7, 2019 12:06
@murarth murarth merged commit 830d47a into rust-windowing:master Dec 7, 2019
@fogti fogti deleted the iss1292evlco branch February 3, 2020 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@goddessfreya goddessfreya goddessfreya approved these changes

@murarth murarth murarth approved these changes

@Osspial Osspial Osspial approved these changes

Assignees
No one assigned
Labels
C - in progress Implementation is proceeding smoothly DS - ios DS - macos DS - wayland DS - web DS - windows DS - x11 S - api Design and usability
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

EventLoopClosed should contain the original event
4 participants
@fogti @murarth @Osspial @goddessfreya