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

Render template columns in pinned rows. #483

Merged
merged 4 commits into from
Oct 7, 2017
Merged

Render template columns in pinned rows. #483

merged 4 commits into from
Oct 7, 2017

Conversation

khirstinova
Copy link
Contributor

No description provided.

@jieter
Copy link
Owner

jieter commented Oct 5, 2017

Py34-master is Django master which does not support python 3.4 anymore since it's the development version of django 2.1.

I'll update the config to reflect that tonight

@jieter
Copy link
Owner

jieter commented Oct 5, 2017

Just pushed the updated tox config and triggered a rebuild.

@jieter
Copy link
Owner

jieter commented Oct 5, 2017

Looking at your implementation: this fixed the problem for TemplateColumn, but what if I use a LinkColumn?
I don't really understand why BoundPinnedRow implements a custom _get_and_render_with method anyway...

@khirstinova
Copy link
Contributor Author

khirstinova commented Oct 5, 2017 via email

@jieter
Copy link
Owner

jieter commented Oct 5, 2017

Your test also passes if I remove the custom method in BoundPinnedRows

@khirstinova
Copy link
Contributor Author

I'm fine with that approach - I don't see anything wrong with it, was just trying to preserve a concern that apparently didn't exist. Would you like me to remove that and check it in?

@jieter
Copy link
Owner

jieter commented Oct 5, 2017

@djk2 Any thoughts on removing BoundPinnedRow._get_and_render_with()?

@djk2
Copy link
Contributor

djk2 commented Oct 6, 2017

I can't remember why I implemented custom method.
Looks that removing _get_and_render_with not break anything.
I think that is a good idea to remove this custom method.

@jieter
Copy link
Owner

jieter commented Oct 6, 2017

@khirstinova do you want to add a commit removing the method?

@jieter jieter merged commit 3f043a6 into jieter:master Oct 7, 2017
@jieter
Copy link
Owner

jieter commented Oct 7, 2017

@khirstinova Just merged, will remove the method in a minute.

Thanks for the pull!

@khirstinova
Copy link
Contributor Author

khirstinova commented Oct 9, 2017 via email

@jieter
Copy link
Owner

jieter commented Oct 10, 2017

released 1.12.0

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.

3 participants