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

Undo changes to the warlock.model_factory API #39

Merged
merged 1 commit into from
Nov 2, 2019

Conversation

nathanielknight
Copy link
Contributor

The changes introduced by commit 64a771d ("Allow
resolver to be set on the Model") added a new keyword argument to the
warlock.model_factory function. Due to the way Python handles
positional arguments, the change alters the way that existing code is
interpreted.

Before the change,

warlock.model_factory(schema_arg, snd_pos_arg)

would execute with

kwargs == {
    'base_class': snd_pos_arg,
    'name': None,
}

but after the change it's interpreted as

kwargs == {
    'resolver': snd_pos_arg,
    'base_class'=model.Model,
    'name': None,
}

This commit re-arranges the arguments to restore the previous behaviour
in an effort to not break existing code.

The changes introduced by commit 64a771d ("Allow
resolver to be set on the Model") added a new keyword argument to the
`warlock.model_factory` function. Due to the way Python handles
positional arguments, the change alters the way that existing code is
interpreted.

Before the change,

    warlock.model_factory(schema_arg, snd_pos_arg)

would execute with

    kwargs == {
        'base_class': snd_pos_arg,
        'name': None,
    }

but after the change it's interpreted as

    kwargs == {
        'resolver': snd_pos_arg,
        'base_class'=model.Model,
        'name': None,
    }

This commit re-arranges the arguments to restore the previous behaviour
in an effort to not break existing code.
@janw janw added this to the 1.4. milestone May 20, 2019
@janw janw merged commit 375c753 into bcwaldon:master Nov 2, 2019
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

2 participants