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

Rename window build methods #453

Merged
merged 10 commits into from
Feb 18, 2020

Conversation

anchnk
Copy link
Contributor

@anchnk anchnk commented Feb 17, 2020

Tracking progress of the methods that should be renamed from window Builder pattern:

  • with_dimensions
  • with_min_dimensions
  • with_max_dimensions
  • with_title
  • with_fullscreen
  • with_maximized
  • with_visibility
  • with_transparency
  • with_decorations
  • with_multitouch

Close #451

This patch renames window builder method with_title to title.
This patch renames window builder method with_dimensions to dimensions.
This patch renames window builder method with_min_dimensions to min_dimensions
This patch renames window builder method with_max_dimensions to max_dimensions.
This patch renames window builder method with_fullscreen to fullscreen.
This patch renames window builder method with_maximized to maximized.
This patch renames window builder method with_visibility to visibility
This patch renames window builder method with_transparency to transparency.
This patch renames window builder method with_decorations to decorations.
This patch renames window builder method with_multitouch to multitouch.
@mitchmindtree
Copy link
Member

Thanks a lot @anchnk! I'll take a look and merge this soon, before I go any further on #452.

@anchnk anchnk changed the title WIP: Rename window build methods Rename window build methods Feb 18, 2020
@anchnk
Copy link
Contributor Author

anchnk commented Feb 18, 2020

Awesome @mitchmindtree! Should we log issues for the missing examples/tests on some building methods?

@mitchmindtree
Copy link
Member

Opening issues for missing examples is a great idea!

W.r.t. tests - I'm not sure how to best go about CI-compatible testing for some of these APIs where they call-down into very platform-specific functionality like window-building, graphics calls etc. I think it's very difficult to come up with something that is both easy to maintain and provides test results that are useful and worth the maintenance cost. That said if you have some ideas on this feel free to open issues!

@anchnk
Copy link
Contributor Author

anchnk commented Feb 18, 2020

I am not sure how to test that either TBH. Having examples that covers these method would already be a big enhancement to ensure things are still working if something changes at some point.

It might sound silly but does a testing strategy such as screenshot diffing could be considered? Such as what's done by Jest in the browser domain for instance. Some kind of headless window where one could access relevant context properties could be another option but as you said it would cost a lot of extra time for very small benefits.

@mitchmindtree
Copy link
Member

Yeah it would be handy if wgpu had a software rendering backend that made it easier to setup automated testing. I think I opened an issue up at the gfx-rs repo a while back about implementing a software implementation, but I don't think it's a trivial job and I'm not sure there's been any action on it.

Anyways, this looks good to me!

@mitchmindtree mitchmindtree merged commit cf77866 into nannou-org:master Feb 18, 2020
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.

Rename window builder methods from with_* to *
2 participants