Skip to content
This repository has been archived by the owner on Nov 14, 2024. It is now read-only.

TupleComparisonFactory is not a factory #2512

Merged
merged 1 commit into from
Oct 18, 2017
Merged

Conversation

hsaraogi
Copy link
Contributor

@hsaraogi hsaraogi commented Oct 18, 2017

Goals (and why): Dont name something thats not a Factory as factory. This causes internal large product tests to fail:

Implementation Description (bullets): Few renames.

Concerns (what feedback would you like?): Is strategy a good name?

Where should we start reviewing?: small change

Priority (whenever / two weeks / yesterday): asap

java.lang.AssertionError: The 1 classes listed below violate the convention: Factories should be objects with methods, not a collection of static methods (LoadLevels over LoadLevelFactory) (enforced by FactoriesAreObjectsNotStaticUtilityClasses)
"com.palantir.atlasdb.keyvalue.dbkvs.impl.ranges.RangePredicateHelper$TupleComparisonFactory"```

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/palantir/atlasdb/2512)
<!-- Reviewable:end -->

Copy link
Contributor

@fsamuel-bs fsamuel-bs left a comment

Choose a reason for hiding this comment

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

LGTM

@hsaraogi hsaraogi force-pushed the fix/naming-convention branch from 6b06b2e to ebad32d Compare October 18, 2017 14:16
@gsheasby gsheasby assigned hsaraogi and unassigned gsheasby Oct 18, 2017
@gsheasby
Copy link
Contributor

Strategy sounds accutate to me - LGTM

@gsheasby
Copy link
Contributor

If changes from within AtlasDB are going to continue to affect LARGE_INTERNAL_PRODUCT in this way, we should add the checks to our repo.

@hsaraogi hsaraogi merged commit d8ea9cd into develop Oct 18, 2017
@hsaraogi hsaraogi deleted the fix/naming-convention branch October 18, 2017 15:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants