Skip to content

Core: Minor code improvements#14489

Merged
singhpk234 merged 1 commit into
apache:mainfrom
nastra:minor-improvements
Nov 3, 2025
Merged

Core: Minor code improvements#14489
singhpk234 merged 1 commit into
apache:mainfrom
nastra:minor-improvements

Conversation

@nastra
Copy link
Copy Markdown
Contributor

@nastra nastra commented Nov 3, 2025

No description provided.

Copy link
Copy Markdown
Contributor

@singhpk234 singhpk234 left a comment

Choose a reason for hiding this comment

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

mostly LGTM, thanks @nastra !

} catch (UnsupportedEncodingException e) {
throw new RuntimeException(e);
}
return URLEncoder.encode(string, StandardCharsets.UTF_8);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

minor : now we don't required to wrap UnsupportedEncodingException in RTE ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

correct, because encode doesn't throw on a known charset (passing "UTF-8" as a plain string would throw in case the encoding isn't supported)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah I see, nice catch :) !

@nastra
Copy link
Copy Markdown
Contributor Author

nastra commented Nov 3, 2025

thanks for reviewing @singhpk234

@singhpk234 singhpk234 merged commit 94d9287 into apache:main Nov 3, 2025
43 checks passed
@nastra nastra deleted the minor-improvements branch November 3, 2025 16:00
thomaschow pushed a commit to thomaschow/iceberg that referenced this pull request Jan 19, 2026
talatuyarer pushed a commit to talatuyarer/iceberg that referenced this pull request Apr 1, 2026
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.

2 participants