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

Use kwargs in policy initializer API #193

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hernanat
Copy link

@hernanat hernanat commented Oct 2, 2021

From #190

This commit changes the policy initializer API to take all kwargs instead of having
an optional first record arg.

From palkan#190

This commit changes the policy initializer API to take all kwargs instead of having
  an optional first `record` arg.
@hernanat
Copy link
Author

hernanat commented Oct 2, 2021

@palkan not sure if any documentation updates are needed, also not sure what section in the changelog to put it under as I wasn't sure if the next release would be 0.6.1 or something else.

@palkan
Copy link
Owner

palkan commented Oct 20, 2021

not sure if any documentation updates are needed, also not sure what section in the changelog to put it under as I wasn't sure if the next release would be 0.6.1 or something else.

Please, add a change log under the ## master version.

This is a breaking change (it was planned for v1). I think, we can include it into v0.x iff:

  • There is a backward compatibility (i.e., we support both positional record and record:, the latter wins if defined)
  • There is a deprecation message saying that new(record) will be removed in v1, use new(record: record) instead.

All internals should use the new kwargs API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants