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

Implement WindowBuilder::with_outer_position for X11 #1189

Merged

Conversation

murarth
Copy link
Contributor

@murarth murarth commented Sep 26, 2019

  • Tested on all platforms changed
  • Compilation warnings were addressed
  • cargo fmt has been run on this branch
  • 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
  • Updated feature matrix, if new features were added or implemented

@murarth murarth requested a review from Osspial October 1, 2019 18:40
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 largely looks good to me.

WRT implementing on other platforms - should we create a new branch to host these changes, then merge that branch once it's implemented everywhere?

src/window.rs Show resolved Hide resolved
@murarth
Copy link
Contributor Author

murarth commented Oct 5, 2019

Merging to a new branch makes sense to me.

Also, I haven't added a changelog entry because I figured we could add one when it's all done instead of one for each platform as they're implemented. If this isn't the way it's usually done, let me know.

@Osspial Osspial changed the base branch from master to with_outer_position October 5, 2019 22:59
@Osspial Osspial merged commit a748b91 into rust-windowing:with_outer_position Oct 5, 2019
@murarth murarth deleted the x11-with-outer-position branch October 6, 2019 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants