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

Refactoring code snippets in quickstart.md #565

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

sanspareilsmyn
Copy link

Description

Refactoring code snippets in quickstart.md. This PR closes #516.

Additional context and related issues

The refactoring of the script enhances readability and maintainability by organizing code into functions and adding clear comments.

Release notes

(x) This is not user-visible or is docs only, and no release notes are required.

@cla-bot cla-bot bot added the cla-signed label Dec 5, 2024
docs/quickstart.md Outdated Show resolved Hide resolved
docs/quickstart.md Outdated Show resolved Hide resolved
docs/quickstart.md Outdated Show resolved Hide resolved
docs/quickstart.md Outdated Show resolved Hide resolved
@sanspareilsmyn
Copy link
Author

@mosabua Thanks for the reivew. I've just applied your feedbacks!


for i in 1 2; do
add_backend "trino$i" "$i"
done }'
Copy link
Member

Choose a reason for hiding this comment

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

End of line is wrong

POSTGRES_SQL="gateway-ha-persistence-postgres.sql"
JAR_FILE="gateway-ha-$VERSION-jar-with-dependencies.jar"
GATEWAY_JAR="gateway-ha.jar"
CONFIG_YAML="quickstart-config.yaml"
Copy link
Member

Choose a reason for hiding this comment

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

The script doesnt actually work .. options:

  • remove the log levels line in the config file log.levels-file: ..
  • add a command here that adds an empty log properties file in the right folder
  • add a command here that adds and empty log.properties file in the current folder and update config for current folder

Copy link
Member

Choose a reason for hiding this comment

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

Personally I think we dont need the log levels in the quickstart .. what do you think @willmostly ?

@mosabua
Copy link
Member

mosabua commented Dec 10, 2024

Also with this whole clean up .. could we add one more config for the folder of the Trino Gateway source code .. the default could be . or so

Then the script could be used in any folder .. we would just have to document to change that variable to an absolute path to the checkout .. or even a relative one from the script directory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Wrap too long code snippet in documentation
2 participants