Do we want to get into Django CBV land?

[Ben Spaulding]

We all know that Django’s class-based views have been polarizing to some degree. I see another pull request, #826, (sans docs and tests) for running search views on CBV’s. In the way of a design decision, is that somewhere we want to go? Do we have good reason for staying with our own views, or should we join the world of CBV’s?

--

Ben

[Jannis Leidel]

Am Donnerstag, 17. Oktober 2013 06:34:21 UTC+2 schrieb Ben Spaulding:

We all know that Django’s class-based views have been polarizing to some degree. I see another pull request, #826, (sans docs and tests) for running search views on CBV’s. In the way of a design decision, is that somewhere we want to go? Do we have good reason for staying with our own views, or should we join the world of CBV’s?

Yeah, I believe Haystack should deprecate the current views and move to use CBVs instead. Even if they are disputed, it's incredibly useful to be able to modify views in the same way as other 3rd party views.

FTR, I've used these clases before: https://github.com/bennylope/elasticstack/blob/master/elasticstack/views.py that don't use a ListView like the above pull request, but a FormMixin instead.

Jannis

[Chris Adams]

Agreed: when this came up in
https://github.com/toastdriven/django-haystack/issues/350 it seemed
like the only real concern was backwards compatibility, which should
be manageable.

Chris

[Ben Spaulding]

Okay, based on feedback here, similar feedback elsewhere, and the number of issues regarding this, moving to Django CBVs may be the way to go.

Daniel, if you are okay with that, I can work with the committer to get docs, tests, and ensure backwards compatibility for #826.

--

Ben

[Daniel Lindsley]

Sorry for the delay, I've been consumed by the Django Dash & the WSGI Wrestle. Wish I could say I was in love with the idea, but I'm not impressed with that PR for a couple reasons.

1. It still seems like some existing installs that override behavior *will* break. There are a non-trival number (think: hundreds) of these. If this PR passes the built-in tests, maybe it's OK & I'm wrong, but we need to verify this.

2. Zero tests despite tons of new behavior & changes. This alone is enough to not ship it.

3. Several spelling errors & typos, not to mention not matching the existing style.

4. No documentation on the changes or how to use them.

IMO, we *do* have to support the new-style CBVs (much as I dislike them). But I feel like that should be an alternate module that we ship alongside (either with something like this becoming the main ``views.py`` & we move the existing module to ``legacy_views.py`` or vice versa). 

Further, what we ship absolutely needs docs & tests. Too many people lean on it to ship something that half works just because there's social pressure for it. And a migration guide for the existing install base needs to be present if we're actually ever going to deprecate the original views. I'm fine with also establishing a deprecation cycle for the old views at the same time (say 2.2 they're deprecated & 2.4 they're out or something, whatever we think we can support).

Sorry to be the bearer of bad news, but there are literally thousands of sites that depend on the these views & we need to support them the best we can.

Daniel

--
You received this message because you are subscribed to the Google Groups "Haystack Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-haystack...@googlegroups.com.
To post to this group, send email to django-ha...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-haystack-dev.
For more options, visit https://groups.google.com/groups/opt_out.

[Jannis Leidel]

On 22.10.2013, at 10:02, Daniel Lindsley <dan...@toastdriven.com> wrote:

> Sorry for the delay, I've been consumed by the Django Dash & the WSGI Wrestle. Wish I could say I was in love with the idea, but I'm not impressed with that PR for a couple reasons.
>
> 1. It still seems like some existing installs that override behavior *will* break. There are a non-trival number (think: hundreds) of these. If this PR passes the built-in tests, maybe it's OK & I'm wrong, but we need to verify this.
> 2. Zero tests despite tons of new behavior & changes. This alone is enough to not ship it.
> 3. Several spelling errors & typos, not to mention not matching the existing style.
> 4. No documentation on the changes or how to use them.
>
> IMO, we *do* have to support the new-style CBVs (much as I dislike them). But I feel like that should be an alternate module that we ship alongside (either with something like this becoming the main ``views.py`` & we move the existing module to ``legacy_views.py`` or vice versa).
>
> Further, what we ship absolutely needs docs & tests. Too many people lean on it to ship something that half works just because there's social pressure for it. And a migration guide for the existing install base needs to be present if we're actually ever going to deprecate the original views. I'm fine with also establishing a deprecation cycle for the old views at the same time (say 2.2 they're deprecated & 2.4 they're out or something, whatever we think we can support).
>
> Sorry to be the bearer of bad news, but there are literally thousands of sites that depend on the these views & we need to support them the best we can.

Actually I don’t think this is bad news, just a reminder that we need to be careful about revamping APIs — especially when it comes to the CBV API.

What if — as a compromise — use the name “SearchFormView” for the new CBV based view? That would fit into the naming convention of CBVs and would allow us to keep only one haystack.views module. I’d rather prevent having a “newviews” module or whatever as it could require changing import twice.

Jannis

[Ben Spaulding]

Like Jannis, I don’t think this is bad news. That PR, though far from perfect, was the best contribution to CBV-migration that we had, so I focused on it. We would not dream of shipping it with out passing tests and docs. But if it does not work for our needs I am happy to close it and start our own.

FWIW, I also like Jannis idea for keeping things in one module. Are there any downsides to that method that you see?

--

Ben

To unsubscribe from this group and stop receiving emails from it, send an email to django-haystack-dev+unsub...@googlegroups.com.

[Daniel Lindsley]

Sorry, I dropped the ball here. I’m in favor of Jannis’ suggestion. Mostly, I just don’t want to break existing users’ installs as much as possible. So if we can dodge that bullet & support the folks clamoring for “official” CBV support (Haystack had CBVs first, #justsayin), all the better. In short, go for it please.

Daniel

To unsubscribe from this group and stop receiving emails from it, send an email to django-haystack...@googlegroups.com.