hi francesco i disagree with your statement below. having tests even for trivial changes just limits the risk of regressions now and for any later modifications... hoping for others having covered your code with some test, is IMO not good enough :-)
so, please add some tests or verify that it's actually covered by some others (intelliJ actually allows you to visualise that pretty easily when running tests with colllecting test-coverage). kind regards angela On 23/10/15 12:13, "Francesco Mari" <[email protected]> wrote: >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
