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

Literal defaults #80

Merged
merged 5 commits into from
Sep 24, 2023
Merged

Conversation

bertramakers
Copy link
Contributor

Hi,

I propose to add a convenience method to directly set a literal default value on fields, instead of using a callback. The callback is nice for scenario's like setting dynamic values based on the Context, but in a lot of cases we just want to set a default "hardcoded" value.

I've added a second method called defaultLiteral() so the choice between using a callback or a literal value is more explicit and there is less chance of accidental errors/bugs in my opinion. And I kept the original default() method so as to not introduce breaking changes. But I'm also open to other solutions if you prefer another approach.

However, I will be traveling the next two weeks and won't have access to my workstation so any changes you'd like you'll either have to implement yourself or wait for a couple of weeks. I will have access to my email/github to exchange messages though.

Thanks for the consideration!

@bertramakers
Copy link
Contributor Author

I'm not sure why the prettier job always fails for my PRs, I think because it's looking for the branch in the original repo (for some reason) while it exists in a fork.

@tobyzerner
Copy link
Owner

Nice idea, thanks for the PR! I would just go with the single default method being able to handle both callbacks and literals. Don't need to worry about breaking changes during alpha ;) Will make this change and get it merged soon. Enjoy your travels!

@tobyzerner tobyzerner merged commit 5b30abe into tobyzerner:main Sep 24, 2023
2 of 3 checks passed
@bertramakers
Copy link
Contributor Author

Thanks for getting this merged @tobyzerner !

Unrelated, but I also just discovered that you have also already provided support for the Atomic Operations extension including support for lid. That makes this library so insanely powerful, I love it!

@bertramakers bertramakers deleted the literal-defaults branch September 25, 2023 09:01
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.

2 participants