Skip to content

Conversation

@tzangms
Copy link
Owner

@tzangms tzangms commented May 3, 2017

No description provided.

@tzangms
Copy link
Owner Author

tzangms commented May 3, 2017

@SunilMohanAdapa @aouaki Is there any change you guys could do code review for me?

@SunilMohanAdapa
Copy link
Contributor

@tzangms I'd be happy to. Will try to do this as soon as find some time (today/tomorrow).

context = {'form': element, 'classes': markup_classes}


if django_version < (1, 11):
Copy link

Choose a reason for hiding this comment

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

This should be < (1, 8) since using Context instances has been deprecated as of Django 1.8

context = Context(context)

if django_version >= (1, 8):
context = context.flatten()
Copy link

Choose a reason for hiding this comment

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

And this has no reason to be anymore

@aouaki
Copy link

aouaki commented May 19, 2017

Reviewed

@JosephKiranBabu
Copy link

@tzangms @aouaki Can this be reviewed and merged soon? This is causing bugs in a dependent projects like Plinth. freedombox/Plinth#900 and freedombox/Plinth#894

@tzangms tzangms merged commit 37baf71 into master Aug 3, 2017
@tzangms tzangms deleted the SunilMohanAdapa-django-1.9 branch August 3, 2017 16:51
@tzangms
Copy link
Owner Author

tzangms commented Aug 3, 2017

Hi All,

Sorry for the late reply, I've did some modify suggested by @aouaki
and released a new version 3.3 on PyPI.

https://pypi.python.org/pypi/django-bootstrap-form/3.3

Please test before put on production, thanks!

@JosephKiranBabu
Copy link

@tzangms Thanks for the quick response.

@JoKeyser
Copy link

JoKeyser commented Aug 10, 2017

Sorry to poke again, but will the merge also solve https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=865938 ? That's the current issue threatening Plinth to be deleted from Debian, I don't have time to look at this right now.

@tzangms
Copy link
Owner Author

tzangms commented Aug 11, 2017

Hi @JoKeyser

I've take a looks at the detail, but is there any document to follow to solve this problem?

@JoKeyser
Copy link

Hi @tzangms , thank you looking into this! And sorry, I also don't know what the best answer is here. Probably @SunilMohanAdapa or @jvalleroy or @petterreinholdtsen know what the best solution is.

In my understanding, the Debian maintainer or another Debian developer(?) would have to finally upload a working (i.e. building-from-source) package to the Debian archive. You (and maybe tomorrow, me) could help by checking whether the problem of bug 865938 would still exist in the current version, now that this PR got merged. Sorry, I just don't have time today.

@tzangms
Copy link
Owner Author

tzangms commented Aug 11, 2017

Oh, I think I miss read the message in Debian bug report page, obviously there is a test error should be fixed, though the unit test is passed in every test on Travis CI.

@JoKeyser
Copy link

JoKeyser commented Aug 12, 2017

I'm sorry, my previous 2 posts were confusing (because I was confused myself).
The Debian issue https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=865938 actually links back to PR #86. As the maintainer, @thomasgoirand says there, the patch is currently necessary to build from source, which is a requirement to remain in Debian (along with all depending package in Debian).
The new Django version 1.11.3 makes another, but very similar patch necessary, which I posted to bug 865938 as an attached patch.
So I think the smartest/easiest way forward would be if PR #86 gets merged and even generalized to also get rid of the similar problem with Django 1.11.3.

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.

6 participants