Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[docs] Update Presto version in example in installation/cli.rst #24491

Merged
merged 1 commit into from
Feb 5, 2025

Conversation

steveburnett
Copy link
Contributor

@steveburnett steveburnett commented Feb 4, 2025

Description

Noticed the command example in installation/cli.rst showed 0.286 instead of 0.290.

As this doc PR, once merged, will be part of the 0.292 release, updated the example command to 0.292.

Screenshot 2025-02-04 at 1 43 11 PM

Motivation and Context

Following the current instructions, the user downloads presto-cli-0.290-executable.jar. The rename command shown in the doc is mv presto-cli-0.286-executable.jar presto which does nothing to rename the downloaded presto-cli-0.290-executable.jar.

If there's an automated way to pick up the release version of Presto and include it inside a code block, let me know, but if not then manual updates are the best current answer.

Impact

Documentation.

Test Plan

Local doc build. See screenshot in Description for local doc build result.

Contributor checklist

  • Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

== NO RELEASE NOTE ==

@steveburnett steveburnett self-assigned this Feb 4, 2025
@steveburnett steveburnett requested review from elharo and a team as code owners February 4, 2025 18:48
@prestodb-ci prestodb-ci added the from:IBM PR from IBM label Feb 4, 2025
@prestodb-ci prestodb-ci requested review from a team, anandamideShakyan and ShahimSharafudeen and removed request for a team February 4, 2025 18:48
@github-actions github-actions bot added the docs label Feb 4, 2025
Copy link
Member

@imjalpreet imjalpreet left a comment

Choose a reason for hiding this comment

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

LGTM.

However, should we mention that it's important to have only one CLI JAR file in the current directory, since we're using the * wildcard now. If there are multiple CLI JAR files present, instead of renaming them, the command will attempt to move all the JARs to a directory named presto

@steveburnett
Copy link
Contributor Author

LGTM.

However, should we mention that it's important to have only one CLI JAR file in the current directory, since we're using the * wildcard now. If there are multiple CLI JAR files present, instead of renaming them, the command will attempt to move all the JARs to a directory named presto

@imjalpreet good catch! I just tested that by downloading multiple CLI jars then manually renaming the extras to earlier version numbers so I had

presto-cli-0.288-executable.jar
presto-cli-0.289-executable.jar
presto-cli-0.290-executable.jar

in the same directory.
Then I ran the command
mv presto-cli-*-executable.jar presto

The command failed and returned the error
mv: presto is not a directory.

Honestly I think that if we need to add a comment to "only [have] one CLI JAR file in the current directory", it might be better for me to put back in the comment

(Replace ``*`` in the example with the version number of the downloaded jar file)

which would help in the case of multiple CLI JAR files.

@tdcmeehan
Copy link
Contributor

SGTM, let's add back in the comment so it's less ambiguous.

@steveburnett steveburnett force-pushed the steveburnett-cli-bump branch from 1a80f10 to 8bffe97 Compare February 5, 2025 16:21
@steveburnett
Copy link
Contributor Author

SGTM, let's add back in the comment so it's less ambiguous.

Done! (Commits squashed and pushed again)

Screenshot 2025-02-05 at 11 21 02 AM

@tdcmeehan tdcmeehan merged commit 1b51466 into prestodb:master Feb 5, 2025
55 checks passed
@steveburnett steveburnett deleted the steveburnett-cli-bump branch February 5, 2025 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs from:IBM PR from IBM
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants