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 form field regression #29

Closed
wants to merge 6 commits into from
Closed

Conversation

akx
Copy link
Contributor

@akx akx commented Feb 23, 2015

Well, damnit.

Looks like my fancy patch broke loading enum values from instances from ModelForms; there wasn't a valid test for it either.

Now there's a fix and while at it, I improved test coverage in general.

@akx
Copy link
Contributor Author

akx commented Feb 23, 2015

Now that Travis is back online, he seems to disagree with the tests. Let's see what I can do to fix that...

@akx akx force-pushed the form-field-regression branch 2 times, most recently from bba3d98 to c681859 Compare February 23, 2015 11:41
@akx
Copy link
Contributor Author

akx commented Feb 23, 2015

The Django 1.4 configurations still fail due to a missing import_string/import_by_path... Grump grump, fixing. :)

@akx
Copy link
Contributor Author

akx commented Feb 23, 2015

phew

That's more like it!

@matthewwithanm
Copy link
Contributor

Brilliant! Thanks so much!!

I feel like a jerk for even mentioning this given your awesome contributions, but generally you shouldn't bump versions in PRs. Usually people want to tag the version after the merge commit and, besides, it can make it harder if other PRs/versions get dealt with in the meantime.

But seriously, this is great. Thanks!

@akx
Copy link
Contributor Author

akx commented Feb 23, 2015

Haha, yeah, I figured it's a 50/50 chance to either get flak for doing the bump in the PR or for it to be convenient for you :) Won't do it next time!

Anyway, you're welcome!

@akx akx deleted the form-field-regression branch February 23, 2015 16:39
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