-
Notifications
You must be signed in to change notification settings - Fork 1
Project housecleaning #5
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.
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
Changes from all commits
cd633b7
f9dc7e9
ced2567
21bd421
4afc46c
9fc57ce
6945838
05be6a2
f845d59
c085638
fcdecda
e6fac6d
514dd52
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
root = true | ||
|
||
[*] | ||
end_of_line = lf | ||
insert_final_newline = true | ||
indent_style = space | ||
indent_size = 4 | ||
charset = utf-8 | ||
continuation_indent_size = 8 | ||
|
||
[*.yml] | ||
indent_style = space | ||
indent_size = 2 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,14 @@ | ||
language: java | ||
|
||
cache: | ||
directories: | ||
- $HOME/.m2 | ||
|
||
jdk: | ||
- openjdk8 | ||
- openjdk9 | ||
- openjdk11 | ||
|
||
matrix: | ||
allow_failures: | ||
- jdk: openjdk9 | ||
- jdk: openjdk11 | ||
- openjdk8 | ||
- openjdk9 | ||
- openjdk11 | ||
|
||
install: true | ||
|
||
script: mvn clean install |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -360,11 +360,11 @@ public void describeTo(Description description) { | |
/** | ||
* The BaseStream must produce exactly the given expected items in order, and no more. | ||
* | ||
* For infinite BaseStreams see {@link #startsWith(T...)} or a primitive stream variant | ||
* For infinite BaseStreams see {@link #startsWith(Object...)} or a primitive stream variant | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this an automatic fix suggested by doclint? Since it specifies There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not an automatic fix, but seems to be the correct way of referencing a generically typed vararg parameter. Using
|
||
* @param expectedMatchers Matchers for the items that should be produced by the BaseStream | ||
* @param <T> The type of items | ||
* @param <S> The type of the BaseStream | ||
* @see #startsWith(T...) | ||
* @see #startsWith(Object...) | ||
* @see #startsWithInt(int...) | ||
* @see #startsWithLong(long...) | ||
* @see #startsWithDouble(double...) | ||
|
@@ -383,11 +383,11 @@ protected boolean matchesSafely(S actual) { | |
/** | ||
* The BaseStream must produce exactly the given expected items in order, and no more. | ||
* | ||
* For infinite BaseStreams see {@link #startsWith(T...)} or a primitive stream variant | ||
* For infinite BaseStreams see {@link #startsWith(Object...)} or a primitive stream variant | ||
* @param expected The items that should be produced by the BaseStream | ||
* @param <T> The type of items | ||
* @param <S> The type of the BaseStream | ||
* @see #startsWith(T...) | ||
* @see #startsWith(Object...) | ||
* @see #startsWithInt(int...) | ||
* @see #startsWithLong(long...) | ||
* @see #startsWithDouble(double...) | ||
|
@@ -675,7 +675,7 @@ boolean remainingItemsEqual(Iterator<T> expectedIterator, Iterator<T> actualIter | |
expectedAccumulator.add(nextExpected); | ||
T nextActual = actualIterator.next(); | ||
actualAccumulator.add(nextActual); | ||
if(Objects.equals(nextExpected, nextActual)) { | ||
if (Objects.equals(nextExpected, nextActual)) { | ||
return remainingItemsEqual(expectedIterator, actualIterator); | ||
} | ||
} | ||
|
@@ -708,7 +708,7 @@ boolean remainingItemsMatch(Iterator<Matcher<T>> expectedIterator, Iterator<T> a | |
expectedAccumulator.add(nextExpected); | ||
T nextActual = actualIterator.next(); | ||
actualAccumulator.add(nextActual); | ||
if(nextExpected.matches(nextActual)) { | ||
if (nextExpected.matches(nextActual)) { | ||
return remainingItemsMatch(expectedIterator, actualIterator); | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to bump up JUnit to 5, but I think bumping hamcrest from 1.x to 2.x is backwards incompatible.
So this is probably a major version bump for this library if this change makes it in. 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, strictly speaking, yes. Though the only parts of Hamcrest used by java-8-matchers are:
These classes are already in Hamcrest 1.3, and the APIs of these have not changed in 2.2. The upgrade to Hamcrest 2.2 did not result in any changes in
src/main/java
. If anybody uses java-8-matchers with Hamcrest 1.3 (typically depends directly on Hamcrest 1.3 as well as java-8-matchers), they will work together.Though there are no mechanisms to tell us if backwards compatibility is broken at a later time, with changes in the library down the road. So the most correct thing may be to bump the major version. But merging this PR will actually not introduce any incompatibility with Hamcrest 1.3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Myself, I really don't have any preference on bumping major version or not, as this is a type of library which is probably mostly, or maybe even always, used as a direct test-dependency, and not pulled in as a transitive dependency of something else. So upgrading
java-8-matchers
is just bumping the version from 1.x to 2, and I don't see the potential of any major version convergence conflicts with other dependencies which themselves may depend on 1.x ofjava-8-matchers
.As I see it, there should be no big deal to bumping the major version for a release after merging this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll probably make a major version bump then, so at least v1 = Hamcrest 1.x, v2 = Hamcrest 2.x
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about merging #6 and #7 first to create a 1.9 release?