impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jin Chul Kim <jinc...@gmail.com>
Subject Re: IMPALA-5078: break up expr-test.cc
Date Thu, 16 Nov 2017 15:47:44 GMT
Sure. Here is a plan to do this carefully.

1. The first change is to move only string part to test-string-expr.cc,
which just move/copy a chunk of code to a new file. There is a redundant
code in the both files between test-expr.cc and test-string-expr.cc because
we hope minimal code change.
2. Iterate #1 approach for the remaining parts. There are several changes.
3. Splitting test-expr.cc is finished.
4. We can find rooms to refactor the new files because there are
redundant/unnecessary code. Several changes are required for refactoring by
an incremental manner.

Best regards,
Jinchul

2017-11-16 23:51 GMT+09:00 Jim Apple <jbapple@cloudera.com>:

> I like this idea.
>
> One thing to be careful of is to not modify the tests themselves when
> you move them. It's hard to see such changes in gerrit and it's hard
> to track them down in git history.
>
> On Thu, Nov 16, 2017 at 5:34 AM, Jin Chul Kim <jinchul@gmail.com> wrote:
> > Hi,
> >
> > https://issues.apache.org/jira/browse/IMPALA-5078
> >
> > I'd like to get your advice if I will refactor expr-test.cc. Henry
> suggests
> > grouping tests by string instructions and move them to
> expr-string-test.cc.
> > I like his idea. You know filenames (e.g. cast-functions*,
> > timestamp-functions*, aggregate-functions*) of backend kernel code in
> exprs
> > have a same pattern, so I would suggest a pair of kernel code and unit
> > test. For example,
> > (cast-functions*, expr-cast-test.cc),
> > (timestamp-functions*, expr-timestamp-test.cc),
> > (aggregate-functions*, expr-aggregate-test.cc),
> > ...
> >
> > Do you have any suggestion or comment on this? Thanks.
> >
> > Best regards,
> > Jinchul
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message