Re: Cleanup after yourselves please

2016-10-18 Thread Josh McKenzie
>
>  tests hastily and messly commented out line by line (*whyy?*)

 Couldn't we use /* */ comments instead of every single line one by one?


When Jake and I were mass porting unit tests for 8099, I know I used idea's
shortcut (ctrl + /) to block comment out things that wouldn't compile while
porting over other tests; multi-line comments break from other multi-line
comments inside/between methods. Unfortunately attribution wasn't retained
on merges so we don't know whether to blame Sylvain, Jake, or myself on the
commented out tests that snuck through in the final patch. =/

Not necessarily a good reason, but at least it is *a* reason.

On Tue, Oct 18, 2016 at 12:04 PM, Michael Kjellman <
mkjell...@internalcircle.com> wrote:

> Gotcha, I didn't know we were actually bringing them back from the dead!
>
> That being said, won't the unit tests need to be re-writtten (or at least
> refactored) after your work? Couldn't we use /* */ comments instead of
> every single line one by one? Given we use source control couldn't we
> remove the dead code and get it from the revision history if we need it in
> the future?
>
> > On Oct 18, 2016, at 8:18 AM, Oleksandr Petrov <
> oleksandr.pet...@gmail.com> wrote:
> >
> > I'm currently working on actually making Super Columns work in CQL
> context.
> > Currently they do not really work[1].
> >
> > It's not a very small piece of work. It was in the pipeline for some
> time,
> > although there most likely were more important things that had to be
> worked
> > on. I understand your disappointment and am sorry you stumbled upon this.
> > But for now you may just disregard the commented tests. My branch is
> going
> > to be ready for review soon.
> >
> > [1] https://issues.apache.org/jira/browse/CASSANDRA-12373
> >
> >
> > On Tue, Oct 18, 2016 at 5:10 PM Michael Kjellman <
> > mkjell...@internalcircle.com> wrote:
> >
> >> There was a bunch of tests hastily and messly commented out line by line
> >> (*whyy?*) ColumnFamilyStoreTest with comments that they are pending
> >> SuperColumns support post 8099.
> >>
> >> Could those responsible please cleanup after themselves? It's been a
> while
> >> since 8099 was committed in the first place and I don't see us adding
> Super
> >> Column support at this point and the unit tests surly will need to be
> >> rewritten anyways.
> >>
> >> As my mother always said, pick your dirty wet towel in the hamper off
> the
> >> floor and put it in the hamper please
> >>
> >> best,
> >> kjellman
> >>
> >> Sent from my iPhone
> >
> > --
> > Alex Petrov
>
>


Re: Cleanup after yourselves please

2016-10-18 Thread Michael Kjellman
Cool, as I would have assumed they would need to be. Given they were initially 
commented out on 6/30/15 maybe cleanup and removal of that dead code is still 
at least warranted.

On Oct 18, 2016, at 9:15 AM, Oleksandr Petrov 
> wrote:

Unit tests will be completely rewritten I suspect.



Re: Cleanup after yourselves please

2016-10-18 Thread Oleksandr Petrov
It makes sense to make them work for backwards compatibility. There was no
announcement that if you migrated to 3.x they wouldn't work, so everyone
would most likely expect a clear upgrade path. it's not bringing them back
from the dead, but rather helping people who want to upgrade to have this
option.

Unit tests will be completely rewritten I suspect.
On Tue, 18 Oct 2016 at 18:04, Michael Kjellman 
wrote:

> Gotcha, I didn't know we were actually bringing them back from the dead!
>
> That being said, won't the unit tests need to be re-writtten (or at least
> refactored) after your work? Couldn't we use /* */ comments instead of
> every single line one by one? Given we use source control couldn't we
> remove the dead code and get it from the revision history if we need it in
> the future?
>
> > On Oct 18, 2016, at 8:18 AM, Oleksandr Petrov <
> oleksandr.pet...@gmail.com> wrote:
> >
> > I'm currently working on actually making Super Columns work in CQL
> context.
> > Currently they do not really work[1].
> >
> > It's not a very small piece of work. It was in the pipeline for some
> time,
> > although there most likely were more important things that had to be
> worked
> > on. I understand your disappointment and am sorry you stumbled upon this.
> > But for now you may just disregard the commented tests. My branch is
> going
> > to be ready for review soon.
> >
> > [1] https://issues.apache.org/jira/browse/CASSANDRA-12373
> >
> >
> > On Tue, Oct 18, 2016 at 5:10 PM Michael Kjellman <
> > mkjell...@internalcircle.com> wrote:
> >
> >> There was a bunch of tests hastily and messly commented out line by line
> >> (*whyy?*) ColumnFamilyStoreTest with comments that they are pending
> >> SuperColumns support post 8099.
> >>
> >> Could those responsible please cleanup after themselves? It's been a
> while
> >> since 8099 was committed in the first place and I don't see us adding
> Super
> >> Column support at this point and the unit tests surly will need to be
> >> rewritten anyways.
> >>
> >> As my mother always said, pick your dirty wet towel in the hamper off
> the
> >> floor and put it in the hamper please
> >>
> >> best,
> >> kjellman
> >>
> >> Sent from my iPhone
> >
> > --
> > Alex Petrov
>
> --
Alex Petrov


Re: Cleanup after yourselves please

2016-10-18 Thread Michael Kjellman
Gotcha, I didn't know we were actually bringing them back from the dead! 

That being said, won't the unit tests need to be re-writtten (or at least 
refactored) after your work? Couldn't we use /* */ comments instead of every 
single line one by one? Given we use source control couldn't we remove the dead 
code and get it from the revision history if we need it in the future?

> On Oct 18, 2016, at 8:18 AM, Oleksandr Petrov  
> wrote:
> 
> I'm currently working on actually making Super Columns work in CQL context.
> Currently they do not really work[1].
> 
> It's not a very small piece of work. It was in the pipeline for some time,
> although there most likely were more important things that had to be worked
> on. I understand your disappointment and am sorry you stumbled upon this.
> But for now you may just disregard the commented tests. My branch is going
> to be ready for review soon.
> 
> [1] https://issues.apache.org/jira/browse/CASSANDRA-12373
> 
> 
> On Tue, Oct 18, 2016 at 5:10 PM Michael Kjellman <
> mkjell...@internalcircle.com> wrote:
> 
>> There was a bunch of tests hastily and messly commented out line by line
>> (*whyy?*) ColumnFamilyStoreTest with comments that they are pending
>> SuperColumns support post 8099.
>> 
>> Could those responsible please cleanup after themselves? It's been a while
>> since 8099 was committed in the first place and I don't see us adding Super
>> Column support at this point and the unit tests surly will need to be
>> rewritten anyways.
>> 
>> As my mother always said, pick your dirty wet towel in the hamper off the
>> floor and put it in the hamper please
>> 
>> best,
>> kjellman
>> 
>> Sent from my iPhone
> 
> -- 
> Alex Petrov



Re: Cleanup after yourselves please

2016-10-18 Thread Oleksandr Petrov
I'm currently working on actually making Super Columns work in CQL context.
Currently they do not really work[1].

It's not a very small piece of work. It was in the pipeline for some time,
although there most likely were more important things that had to be worked
on. I understand your disappointment and am sorry you stumbled upon this.
But for now you may just disregard the commented tests. My branch is going
to be ready for review soon.

[1] https://issues.apache.org/jira/browse/CASSANDRA-12373


On Tue, Oct 18, 2016 at 5:10 PM Michael Kjellman <
mkjell...@internalcircle.com> wrote:

> There was a bunch of tests hastily and messly commented out line by line
> (*whyy?*) ColumnFamilyStoreTest with comments that they are pending
> SuperColumns support post 8099.
>
> Could those responsible please cleanup after themselves? It's been a while
> since 8099 was committed in the first place and I don't see us adding Super
> Column support at this point and the unit tests surly will need to be
> rewritten anyways.
>
> As my mother always said, pick your dirty wet towel in the hamper off the
> floor and put it in the hamper please
>
> best,
> kjellman
>
> Sent from my iPhone

-- 
Alex Petrov


Cleanup after yourselves please

2016-10-18 Thread Michael Kjellman
There was a bunch of tests hastily and messly commented out line by line 
(*whyy?*) ColumnFamilyStoreTest with comments that they are pending 
SuperColumns support post 8099. 

Could those responsible please cleanup after themselves? It's been a while 
since 8099 was committed in the first place and I don't see us adding Super 
Column support at this point and the unit tests surly will need to be rewritten 
anyways. 

As my mother always said, pick your dirty wet towel in the hamper off the floor 
and put it in the hamper please

best,
kjellman

Sent from my iPhone