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

Knob: type deduction. #342

Merged
merged 1 commit into from
Nov 14, 2023
Merged

Knob: type deduction. #342

merged 1 commit into from
Nov 14, 2023

Conversation

hysw
Copy link
Collaborator

@hysw hysw commented Nov 14, 2023

Knob types are defined in header already,
let compiler deduce the type from knob variable
when initializing them.

@hysw
Copy link
Collaborator Author

hysw commented Nov 14, 2023

I'm creating this since I failed copy-pasting knob type twice today.

@hysw hysw changed the title Knob: type deduction for create/init. Knob: type deduction. Nov 14, 2023
@hysw
Copy link
Collaborator Author

hysw commented Nov 14, 2023

I'm going to wait for #343 to be reviewed and submitted, for the sake of not dealing with a bunch of local merge conflict.

@Keenuts
Copy link
Member

Keenuts commented Nov 14, 2023

Note: this is 2 PRs: one is doing type deduction, the other changing the implementation of the color sliders in the sample.

Knob types are defined in header already,
let compiler deduce the type from knob variable
when initializing them.
@hysw
Copy link
Collaborator Author

hysw commented Nov 14, 2023

Note: this is 2 PRs: one is doing type deduction, the other changing the implementation of the color sliders in the sample.

Fair point, removed the second commit, I'll make a different PR.

@hysw hysw mentioned this pull request Nov 14, 2023
Copy link
Collaborator

@angela28chen angela28chen left a comment

Choose a reason for hiding this comment

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

Thanks Peter! I think this would definitely make knobs easier to use.

@Keenuts Keenuts merged commit ad912d0 into google:main Nov 14, 2023
5 checks passed
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.

4 participants