Skip to content

fix a number of small warnings and formatting issues - #10

Merged
elharo merged 3 commits into
masterfrom
xml2
Mar 23, 2020
Merged

elharo merged 3 commits into
masterfrom
xml2

Conversation

@elharo

@elharo elharo commented Feb 23, 2020

Copy link
Copy Markdown
Contributor

@elharo
elharo requested a review from Tibor17 February 23, 2020 16:23
, text
, is( expectedContent ) );
try (Scanner base = new Scanner( expandedFile );
Scanner scanner = base.useDelimiter( "\n" ) )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Having two local variables with the same reference is not necessary. It is the flow design:
https://docs.oracle.com/javase/7/docs/api/java/util/Scanner.html#useDelimiter(java.util.regex.Pattern)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was an interesting corner case I'd not encountered before and it took a while to figure out. It cures a warning about failure to close the first scanner in try-with-resources block. Though looking at it now I see another way to approach this.

@elharo elharo left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

PTAL

@elharo elharo left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ping. Can I move forward with this one?

@rfscholte

Copy link
Copy Markdown
Contributor

@elharo

elharo commented Mar 21, 2020 via email

Copy link
Copy Markdown
Contributor Author

@elharo elharo left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Can I get a formal approval on this?

@elharo
elharo merged commit 001baa7 into master Mar 23, 2020
@elharo
elharo deleted the xml2 branch March 23, 2020 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants