git.net

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Reviews please


I've done a scan of the code and it looks great to me. Also compiled
and ran tests on my machine just in case. I love commits that delete
more code than they add (over 2,600 deleted lines!) I did have a
couple comments that were mostly just clarifications that I added on
the relevant commit. Thanks for this Julian! I know making these kinds
of sweeping changes can be pretty dreary.

I noticed you had a couple suggestions in your commit message for
future changes. Should there be JIRA issues logged for these? (If
there haven't been already). Also, with all the new deprecations that
have been added (which I agree with), I wonder if it's time to start
thinking about planning a 2.0 release?

In any case, since the changes are so pervasive, it would be good to
get this merged as soon as possible to avoid conflicts with future
PRs. Thanks again Julian!

--
Michael Mior
mmior@xxxxxxxxxx
Le sam. 7 juil. 2018 à 16:29, Julian Hyde <jhyde@xxxxxxxxxx> a écrit :
>
> I have two major changes ready to check in. Can someone please review them?
>
> * https://issues.apache.org/jira/browse/CALCITE-2280 Babel SQL parser
> * https://issues.apache.org/jira/browse/CALCITE-2259 Java 8 syntax
>
> Both are staged in the same branch -
> https://github.com/julianhyde/calcite/commits/2280-babel - but it
> could be different people reviewing each.
>
> I am especially interested in high-level comments about design and
> development practices.
>
> Julian