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.

Sign up for GitHub

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

Jump to bottom

ISPN-14575 Remove properties attribute from indexing configuration #10758

Merged
merged 4 commits into from Apr 14, 2023
Merged

ISPN-14575 Remove properties attribute from indexing configuration #10758

merged 4 commits into from Apr 14, 2023

Conversation

fax4ever
Copy link
Contributor

@fax4ever fax4ever commented Mar 31, 2023 •
edited

@tristantarrant
Copy link
Member

Instead of the dychotomy of autoIndexing and manualIndexing using booleans, I'd prefer the use of an enum IndexingMode with MANUAL and AUTO values for now.

@fax4ever
Copy link
Contributor Author

Instead of the dychotomy of autoIndexing and manualIndexing using booleans, I'd prefer the use of an enum IndexingMode with MANUAL and AUTO values for now.

it looks a good idea, I'm going to apply it

@fax4ever
Copy link
Contributor Author

Thanks for the review @tristantarrant.
Did the changes.

@fax4ever
Copy link
Contributor Author

@tristantarrant I also added this commit 2ca9c49 to review the remaining deprecated / undocumented property

tristantarrant
tristantarrant reviewed Apr 14, 2023
View reviewed changes
core/src/main/java/org/infinispan/configuration/cache/IndexingMode.java Outdated
MANUAL;

public static IndexingMode requireValid(String value, Log logger) {
return Arrays.stream(IndexingMode.values())
Copy link
Member

Choose a reason for hiding this comment

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

Easier to do:

try {
return IndexingMode.valueOf(value.toUpperCase());
} catch (IllegalArgumentException e) {
   throw Log.CONFIG.illegalEnumValue(value, EnumSet.allOf(IndexingMode.class));
}

tristantarrant
tristantarrant reviewed Apr 14, 2023
View reviewed changes
core/src/main/java/org/infinispan/configuration/cache/IndexShardingConfiguration.java Outdated

public class IndexShardingConfiguration extends ConfigurationElement<IndexShardingConfiguration> {

public static final AttributeDefinition<Integer> NUMBER_OF_SHARDS =
Copy link
Member

Choose a reason for hiding this comment

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

I think it would nicer to rename this to SHARDS everywhere

tristantarrant
tristantarrant requested changes Apr 14, 2023
View reviewed changes
Copy link
Member

@tristantarrant tristantarrant left a comment

Choose a reason for hiding this comment

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

A couple more changes, please

@fax4ever
Copy link
Contributor Author

A couple more changes, please

Good ideas. I applied the changes

@tristantarrant tristantarrant merged commit c0bf2a4 into infinispan:main Apr 14, 2023
3 of 4 checks passed
@tristantarrant
Copy link
Member

Merged, thanks

@fax4ever fax4ever deleted the ISPN-14575 branch April 14, 2023 10:57
@fax4ever
Copy link
Contributor Author

Thanks @tristantarrant

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

@tristantarrant tristantarrant tristantarrant requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@fax4ever @tristantarrant