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

Handle non-default kwargs in deconstruct #207

Merged
merged 4 commits into from Apr 5, 2014

Conversation

Projects
None yet
2 participants
@blueyed
Copy link
Contributor

commented Mar 13, 2014

According to the documentation [1], non-default kwargs need to get
returned.

1: https://docs.djangoproject.com/en/dev/howto/custom-model-fields/#field-deconstruction

Handle non-default kwargs in deconstruct
According to the documentation [1], non-default kwargs need to get
returned.

1: https://docs.djangoproject.com/en/dev/howto/custom-model-fields/#field-deconstruction
@apollo13

This comment has been minimized.

Copy link
Contributor

commented Mar 13, 2014

@blueyed Shouldn't django/django@6b07804 take care of this?

@blueyed

This comment has been minimized.

Copy link
Contributor Author

commented Mar 13, 2014

@apollo13
django/django@6b07804 only handles rel.through (i.e. the one from TaggableRel), but TaggableManager's through is different.

Any idea why the tests are failing for Django < 1.7?

CommandError: One or more models did not validate:
tests.multipletagsgfk: 'tags2' is a manually-defined m2m relation through model ThroughGFK, which does not have foreign keys to Tag and MultipleTagsGFK

blueyed added some commits Mar 20, 2014

Merge branch 'develop' into deconstruct-keep-non-defaults
This probably fixes the travis builds.

Conflicts:
	taggit/managers.py
Handle "to" with deconstruction
This accepts "to" as kwarg, and provides it via `deconstruct`, and also
changes `deconstruct` to always provide/set "through".

This is more explicit and since the manager always uses a through model
internally, this describes it better.
@apollo13

This comment has been minimized.

Copy link
Contributor

commented Mar 26, 2014

This fails on everything but 1.7

@blueyed

This comment has been minimized.

Copy link
Contributor Author

commented Mar 26, 2014

Ah, the test itself is failing:

Traceback (most recent call last):
AttributeError: 'super' object has no attribute 'deconstruct'

How should this get handled? Skipping the test for Django < 1.7?

@apollo13

This comment has been minimized.

Copy link
Contributor

commented Apr 4, 2014

Jupp, noone needs deconstruct on 1.6 ;)

@blueyed

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2014

Hmm, travis does not appear to pickup this commit/update.

I guess I should rebase the commits anyway, if it looks OK for you in general?!

@apollo13 apollo13 merged commit 8fb7cfa into jazzband:develop Apr 5, 2014

@apollo13

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2014

It didn't pick it up cause it had merge conflicts.

@blueyed blueyed deleted the blueyed:deconstruct-keep-non-defaults branch Apr 20, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.