[VOTE] Release sqlline-1.6.0 (release candidate 0)

[Julian Hyde]

Hi all,

I have created a build for SQLLine 1.6.0, release candidate 0.

Thanks to everyone who have contributed to this release. You can read the release notes here:
https://github.com/julianhyde/sqlline/blob/branch-1.6/HISTORY.md

The commit to be voted upon:
https://github.com/julianhyde/sqlline/commit/42252c4d49bc94efbc515973f7b4c2da1593882d

Its hash is 42252c4d49bc94efbc515973f7b4c2da1593882d

A staged Maven repository is available for review at:
https://oss.sonatype.org/content/repositories/sqlline-1007/

Please vote on releasing this package as SQLLine 1.6.0. The vote is open for the next 72 hours and passes if a majority of at least three +1 votes are cast by committers. Votes from people who are not committers are also most welcome.

[ ] +1 Release this package as SQLLine 1.6.0
[ ] 0 I don't feel strongly about it, but I'm okay with the release
[ ] -1 Do not release this package because…

Here is my vote: +1 (binding)

Julian

[Andrew Pilloud]

+1

Tested against the Beam JDBC driver, looks like it works. Ran into "Bad history file syntax!", which went away when I deleted my history file.

Andrew

[Sergey Nuyanzin]

+1

   Checked checksums

   Compiled under Windows 10 and Fedora 28/29
   Compiled Apache Calcite with sqlline 1.6.0
   Tested with Apache Calcite, Apache Hive, H2
   Tested highlighting, dynamic prompts, multiline editing, line continuation, emacs/vi modes, rerun command, simple queries

[Arina Yelchiyeva]

Build from source on Centos7. Ran unit tests, had to drop history file to get successful run.

Checked with Apache Drill. Overall looks good.

Noticed two issues:

1. maxColumnWidth default is 15. Though I have made it -1 in my PR.

2. Getter for maxWidth is absent in SqlLineOpts, though was present in previous SqlLine version.

In TableOutputFormat to get this property sqlLine.getOpts().getInt(BuiltInProperty.MAX_WIDTH) is used, again previously getter was used.

Kind regards,

Arina

[Julian Hyde]

Sergey,

Can you please comment on Arina’s observations? If they are non-trivial I am happy to cancel this vote and create a new RC.

Arina,

When you say "Though I have made it -1 in my PR.”, is this a PR you made recently?

Julian

-- 
You received this message because you are subscribed to the Google Groups "sqlline-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sqlline-dev...@googlegroups.com.
To post to this group, send email to sqlli...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sqlline-dev/39958957-c830-44da-8dbe-534df936e3b4%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Sergey Nuyanzin]

Arina, thank you for findings.

If I understand correctly

> 1. maxColumnWidth default is 15. Though I have made it -1 in my PR.

there was a PR https://github.com/julianhyde/sqlline/pull/170 where default value of maxColumnWidth changed  from 15 to -1,

but I guess after https://github.com/julianhyde/sqlline/pull/174/files became again 15.

This could impact table outputformat: column width could be a little different in compare with previous versions, while in case -1 value it will be the same as for previous.

> Getter for maxWidth is absent in SqlLineOpts, though was present in previous SqlLine version.

> In TableOutputFormat to get this property sqlLine.getOpts().getInt(BuiltInProperty.MAX_WIDTH) is used, again previously getter was used.

also connected to https://github.com/julianhyde/sqlline/pull/174/files where getter for maxWidth property missed

It looks like a technical debt but can't see any impact on end users (please correct me if I am wrong)

I do not know if we have to cancel this vote or not because of this

but if we cancel it would make sense to fix both as both are trivial to fix

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlline-dev/85ACEC9B-3D8D-4B7D-914A-D0BF32B876DE%40apache.org.

For more options, visit https://groups.google.com/d/optout.

--

Best regards,
Sergey

[Julian Hyde]

Re the getter. We shouldn’t be removing public APIs. (It is difficult to know which are public APIs, and I tend to assume APIs are private unless there is evidence to the contrary — e.g. someone like Arina hollering.) So, the right thing to do is add it back, but mark it deprecated. Then we can remove it next release.

Re maxColumnWidth. I don’t understand the issue, and don’t know whether new behavior is better or worse or simply different.

Re the release. Since there’s at least one bug with an easy fix, let’s make another RC. I’ll send a cancel.

Julian

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlline-dev/CAKQOUBjPBmMwOMRXH5zteLgonMcFeti-JaS5WuMYa4v6SxExaw%40mail.gmail.com.

[Julian Hyde]

Canceling the vote, due to issues found by Arina (see the voting thread for details).

Thanks to all who voted.

Arina, can you log a bug, please?

Julian

[Arina Yelchiyeva]

Prior maxColumnWidth was not used during column width calculation and had default value of 15.

After my PR (https://github.com/julianhyde/sqlline/pull/170), it started to be used in the calculation. I have changed default value to -1 to leave prior behavior. 

Once it was changed to 15 again, all columns max width became 15.

Regarding getter, I have noticed it since I have used it in the unit test, I don't think this one is critical but we definitely should update the default for maxColumnWidth.

I have opened the issue - https://github.com/julianhyde/sqlline/issues/218