Skip to content

Replace var usages with explciit type#12787

Merged
findepi merged 1 commit intotrinodb:masterfrom
findepi:findepi/replace-var-usages-with-explciit-type-cfd35d
Jun 13, 2022
Merged

Replace var usages with explciit type#12787
findepi merged 1 commit intotrinodb:masterfrom
findepi:findepi/replace-var-usages-with-explciit-type-cfd35d

Conversation

@findepi
Copy link
Copy Markdown
Member

@findepi findepi commented Jun 10, 2022

Per development guidelines, `var` is discouraged.
@findepi findepi added maintenance Project maintenance task no-release-notes This pull request does not require release notes entry labels Jun 10, 2022
@findepi findepi requested review from hashhar and losipiuk June 10, 2022 09:13
@cla-bot cla-bot bot added the cla-signed label Jun 10, 2022
@findepi findepi requested a review from ebyhr June 11, 2022 20:31
{
var executionBinder = newMapBinder(binder, new TypeLiteral<Class<? extends Statement>>() {}, new TypeLiteral<QueryExecutionFactory<?>>() {});
MapBinder<Class<? extends Statement>, QueryExecutionFactory<?>> executionBinder =
newMapBinder(binder, new TypeLiteral<Class<? extends Statement>>() {}, new TypeLiteral<QueryExecutionFactory<?>>() {});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: How about replacing types with <> here and L177?

newMapBinder(binder, new TypeLiteral<>() {}, new TypeLiteral<>() {});

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

the idea of TypeLiteral<T>() {} is to capture the T (hence anonymous subclass). It should be explicit.

@findepi findepi merged commit 0f68de8 into trinodb:master Jun 13, 2022
@findepi findepi deleted the findepi/replace-var-usages-with-explciit-type-cfd35d branch June 13, 2022 09:39
@github-actions github-actions bot added this to the 386 milestone Jun 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed maintenance Project maintenance task no-release-notes This pull request does not require release notes entry

Development

Successfully merging this pull request may close these issues.

2 participants