Skip to content

Refactoring the code #677

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Akanksha-kedia
Copy link

Description

Refactoring the code.

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Apr 18, 2025
@Akanksha-kedia
Copy link
Author

@mosabua please review.

@Akanksha-kedia
Copy link
Author

[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for trino-gateway-parent 11-SNAPSHOT:
[INFO]
[INFO] trino-gateway-parent ............................... SUCCESS [ 23.512 s]
[INFO] gateway-ha ......................................... SUCCESS [01:49 min]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 02:14 min
[INFO] Finished at: 2025-04-18T12:10:36+05:30
[INFO] ------------------------------------------------------------------------

@GET
@Path("logo.svg")
public Response getLogo(@Context ServletContext servletContext)
throws IOException
{
String fullPath = "/static/logo.svg";
String fullPath = LOGO_PATH;
Copy link
Member

Choose a reason for hiding this comment

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

This change reduces readability because we have to look up the constants.

message.append("==========");
logger.error("%s", message);
System.exit(100);
handleConfigurationException(e);
Copy link
Member

Choose a reason for hiding this comment

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

What is the motivation of this change?

@@ -94,6 +86,18 @@ private void start(List<Module> additionalModules, HaGatewayConfiguration config
logger.info("======== SERVER STARTED ========");
}

private void handleConfigurationException(ApplicationConfigurationException e)
Copy link
Member

Choose a reason for hiding this comment

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

It's hard to imagine what happens from this method name. We could rename it to logConfigurationException and move System.exit(100) to the caller.

@ebyhr
Copy link
Member

ebyhr commented Apr 19, 2025

Please follow the commit message guideline https://trino.io/development/process.html#pull-request-and-commit-guidelines-

Use the imperative mood, as used in a command or request, in the subject line.

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.

2 participants