Skip to content

Conversation

@astefan
Copy link
Contributor

@astefan astefan commented Nov 5, 2018

This adds a new logo for the SQL CLI representing the Elastic logo "painted" with "Elastic" letters.

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

Copy link
Contributor

@matriv matriv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool!!

@nik9000
Copy link
Member

nik9000 commented Nov 5, 2018

Cute.

@astefan
Copy link
Contributor Author

astefan commented Nov 6, 2018

@costin @matriv I've added the CLI version on the last line of the logo, centered under "SQL". In the process I had to change some bits in the embedded CLI used for testing. If you could have another look, that would be great.

try (BufferedReader reader = new BufferedReader(new InputStreamReader(in, StandardCharsets.UTF_8))) {
String line;
while ((line = reader.readLine()) != null) {
if (line.length() > lineLength) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use Math.max() to avoid the if.

boolean isLogoOrException = false;
while (!isLogoOrException) {
String line = readLine();
isLogoOrException = line.contains("SQL");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can check with "SQL".equals(line.trim()) to avoid the isLogoOrException.

@matriv
Copy link
Contributor

matriv commented Nov 6, 2018

Left a couple of suggestions.

Copy link
Contributor

@matriv matriv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@costin costin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Next stop, ANSI colors :)

}

// print the version centered on the last line
char[] whitespaces = new char[lineLength / 2 - Version.CURRENT.version.length() / 2];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To minimize rounding errors use (lineLength-Version.CURRENT.version.length())/2. It's also shorter.

@astefan astefan merged commit c3e4575 into elastic:master Nov 6, 2018
astefan added a commit that referenced this pull request Nov 6, 2018
Added new SQL CLI logo representing the Elastic logo "painted" with
"Elastic" words, "SQL" under the logo and version on the last line
@astefan astefan deleted the sql_elastic_logo branch November 6, 2018 16:36
matarrese added a commit to matarrese/elasticsearch that referenced this pull request Nov 6, 2018
…-agg

* master: (528 commits)
  Register Azure max_retries setting (elastic#35286)
  add version 6.4.4
  [Docs] Add painless context details for bucket_script (elastic#35142)
  Upgrade jline to 3.8.2 (elastic#35288)
  SQL: new SQL CLI logo (elastic#35261)
  Logger: Merge ESLoggerFactory into Loggers (elastic#35146)
  Docs: Add section about range query for range type (elastic#35222)
  [ILM] change remove-policy-from-index http method from DELETE to POST (elastic#35268)
  [CCR] Forgot missing return statement,
  SQL: Fix null handling for AND and OR in SELECT (elastic#35277)
  [TEST] Mute ChangePolicyForIndexIT#testChangePolicyForIndex
  Serialize ignore_throttled also to 6.6 after backport
  Check for java 11 in buildSrc (elastic#35260)
  [TEST] increase await timeout in RemoteClusterConnectionTests
  Add missing up-to-date configuration (elastic#35255)
  Adapt Lucene BWC version
  SQL: Introduce Coalesce function (elastic#35253)
  Upgrade to lucene-8.0.0-snapshot-31d7dfe6b1 (elastic#35224)
  Fix failing ICU tests (elastic#35207)
  Prevent throttled indices to be searched through wildcards by default (elastic#34354)
  ...
pgomulka pushed a commit to pgomulka/elasticsearch that referenced this pull request Nov 13, 2018
Added new SQL CLI logo representing the Elastic logo "painted" with
"Elastic" words, "SQL" under the logo and version on the last line
@jimczi jimczi removed the v7.0.0 label Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants