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 #308 -- do not force defaults for make and prepare #330

Merged
merged 4 commits into from Aug 2, 2022

Conversation

amureki
Copy link
Collaborator

@amureki amureki commented Jun 30, 2022

PR #292 introduced explicit defaults for make and prepare,
which were then overwriting custom values in certain cases (explained in #308).

This PR attempts to fix this issue by not having explicit defaults.

PR #292 introduced explicit defaults for `make` and `prepare`,
which were then overwriting custom values in certain cases (explained in #308).

This PR attempts to fix this issue by not having explicit defaults.
@amureki amureki self-assigned this Jun 30, 2022
Copy link
Collaborator

@timjklein36 timjklein36 left a comment

Choose a reason for hiding this comment

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

Looks good! (Sorry for the delayed review)

tests/generic/models.py Outdated Show resolved Hide resolved
Copy link
Member

@berinhard berinhard left a comment

Choose a reason for hiding this comment

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

Besides the super syntax, I'm good with the PR! Thanks for opening it

@amureki
Copy link
Collaborator Author

amureki commented Aug 2, 2022

@timjklein36 @berinhard thank you for the reviews!
I will merge dependency updates and will release a new version soon.

@amureki amureki merged commit 8450e82 into main Aug 2, 2022
@amureki amureki deleted the issues/308/do-not-enforce-defaults branch August 2, 2022 14:51
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.

None yet

3 participants