I don't think that in this case a test is necessary, given the triviality of the change. Type is used by almost the whole stack, and the fixes are about the comparison between types. If I introduced a regression, it should have been caught by at least one test (hopefully).
2015-10-23 12:08 GMT+02:00 Chetan Mehrotra <[email protected]>: > On Fri, Oct 23, 2015 at 3:15 PM, <[email protected]> wrote: >> public final class Type<T> implements Comparable<Type<?>> { >> >> - private static final Map<String, Type<?>> TYPES = newHashMap(); >> + private static final Map<String, Type<?>> TYPES = new HashMap<String, >> Type<?>>(); >> >> private static <T> Type<T> create(int tag, boolean array, String >> string) { >> Type<T> type = new Type<T>(tag, array, string); >> @@ -242,10 +237,23 @@ public final class Type<T> implements Co >> >> @Override >> public int compareTo(@Nonnull Type<?> that) { >> - return ComparisonChain.start() >> - .compare(tag, that.tag) >> - .compareFalseFirst(array, that.array) >> - .result(); >> + if (tag < that.tag) { >> + return -1; >> + } >> + >> + if (tag > that.tag) { >> + return 1; >> + } >> + >> + if (!array && that.array) { >> + return -1; >> + } >> + >> + if (array && !that.array) { >> + return 1; >> + } >> + >> + return 0; >> } > > I am fine with removing dependency on Guava from API but not sure if > we should remove it from implementation side > > Also it would be good to have a testcase to validate the above logic > if we are removing dependency from a tested Guava utility method > > Chetan Mehrotra
