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

fix: allow overriding strategy defaults #766

Merged
merged 2 commits into from
Aug 18, 2024

Conversation

zachdaniel
Copy link
Collaborator

auto_set_fields doesn't set defaults, it sets the values always and so things like authorization_params were not settable

@zachdaniel zachdaniel force-pushed the allow-overriding-strategy-defaults branch 3 times, most recently from 6e96586 to e93a9ac Compare August 11, 2024 12:47
@zachdaniel
Copy link
Collaborator Author

@jimsynz i was having trouble locally with asdf so I updated the .tool-versions file. But now we have what I think are unrelated dialyzer errors. However something strange is happening with dialyzer locally, I think because an explicit PLT file is configured? What is the reason for that? Could you take a look at the failures?

zachdaniel and others added 2 commits August 19, 2024 09:33
`auto_set_fields` doesn't set defaults, it sets the values *always*
and so things like `authorization_params` were not settable

fixes #764
@jimsynz jimsynz force-pushed the allow-overriding-strategy-defaults branch from e93a9ac to 02f8a85 Compare August 18, 2024 21:47
@jimsynz
Copy link
Collaborator

jimsynz commented Aug 18, 2024

@zachdaniel dialyzer issues fixed.

@zachdaniel zachdaniel merged commit c4f5703 into main Aug 18, 2024
17 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.

2 participants