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

Add direct builder methods #3066

Merged
merged 3 commits into from
Jan 27, 2024
Merged

Add direct builder methods #3066

merged 3 commits into from
Jan 27, 2024

Conversation

madsmtm
Copy link
Member

@madsmtm madsmtm commented Aug 30, 2023

Add methods EventLoop::builder and Window::builder, which allows creating the related builders without having to import them. See the examples for how things look cleaner.

Unsure about whether we should deprecate the old methods or not?

  • 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

@madsmtm madsmtm added S - api Design and usability C - needs discussion Direction must be ironed out labels Aug 30, 2023
@madsmtm madsmtm requested a review from kchibisov as a code owner August 30, 2023 09:04
@madsmtm madsmtm removed the request for review from kchibisov August 30, 2023 09:15
@madsmtm madsmtm mentioned this pull request Aug 30, 2023
5 tasks
@kchibisov
Copy link
Member

What's the motivation behind those changes though, besides import? I'm not sure I really get the motivation behind all of that.

@madsmtm
Copy link
Member Author

madsmtm commented Sep 1, 2023

Only motivation is to reduce the number of imports / things the user has to think about, and to hopefully make it easier to see at a glance which type you're constructing.

@kchibisov
Copy link
Member

But it's not like you can't request a builder on your own and it's not like we don't have default, so users will still be able to import/construct it?

@madsmtm
Copy link
Member Author

madsmtm commented Sep 1, 2023

Definitely true, and there's no other point to deprecating the old methods other than keeping a slightly cleaner API

Copy link
Member

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

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

Thanks @madsmtm, the stuff you've done lately improves the API a lot!

@kchibisov
Copy link
Member

Needs rebase.

@madsmtm madsmtm requested a review from notgull as a code owner January 27, 2024 18:11
@notgull notgull merged commit 3830b49 into master Jan 27, 2024
51 checks passed
@notgull notgull deleted the add-builders branch January 27, 2024 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C - needs discussion Direction must be ironed out S - api Design and usability
Development

Successfully merging this pull request may close these issues.

4 participants