Recent Remember Category Changes

["David E. Jones" <jo...@hotwaxmedia.com>]

For anyone interested in the recent changes to attempt to do this,  
please see my comments in the commit log and source file changes for  
SVN rev 509955.

This really isn't cool, BTW. The first approach was bad, the second  
one that I just commented out in SVN is worse because of bad side  
effects. This is a lower priority feature than, say, having the  
correct category show on the _main_ page in ecommerce... isn't it?

There are ways to do this properly, like we do with the add to cart.

For this and in general: a poorly implemented approach with bad side  
effects, and dangerous practices like having code in different places  
try to maintain and use session variables, is NOT acceptable.  
Especially for something like this which is a BEST a nice to have.

PLEASE be careful! This isn't what I like to see at 3 AM when trying  
to test and get things working. Especially when it is something I  
shot down on a commit review, and then something just as bad or  
perhaps worse replaced it.

YES! It is better to just leave the feature out if it breaks existing  
and more important functionality. Some patches really should just be  
rejected, and we don't have to feel bad about that.

-David

Re: Recent Remember Category Changes

[Jacques Le Roux <ja...@les7arts.com>]

David,


>
> For anyone interested in the recent changes to attempt to do this,
> please see my comments in the commit log and source file changes for
> SVN rev 509955.

I have reverted product.bsh too and added a comment in all concerned files about the best practice to use : same than with add to
cart (in rev 509990)

> This really isn't cool, BTW. The first approach was bad, the second
> one that I just commented out in SVN is worse because of bad side
> effects. This is a lower priority feature than, say, having the
> correct category show on the _main_ page in ecommerce... isn't it?

Yes I agree.

> There are ways to do this properly, like we do with the add to cart.

I will have a look a this, thanks for the tip.

> For this and in general: a poorly implemented approach with bad side
> effects, and dangerous practices like having code in different places
> try to maintain and use session variables, is NOT acceptable.
> Especially for something like this which is a BEST a nice to have.
>
> PLEASE be careful! This isn't what I like to see at 3 AM when trying
> to test and get things working. Especially when it is something I
> shot down on a commit review, and then something just as bad or
> perhaps worse replaced it.

Yes I'm really sorry. I tried this feature seriously but forgot about the main link which is nevertheless so obvious !

> YES! It is better to just leave the feature out if it breaks existing
> and more important functionality. Some patches really should just be
> rejected, and we don't have to feel bad about that.

Actually, I was more interested by coming back to the same page after changing language than correcting the bug Baga found. So it's
really my mistake, I apologize.

Jacques

> -David
>
>
>