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

Fix warnings in eclipse #1432

Merged
merged 1 commit into from May 9, 2015

Conversation

Projects
None yet
3 participants
@TheJulianJES
Contributor

TheJulianJES commented May 9, 2015

This fixes warnings in eclipse, yes some people are still using eclipse :)
If the review is accepted I will also create a PR for rv2 if wanted.

@thatsIch

This comment has been minimized.

Show comment
Hide comment
@thatsIch

thatsIch May 9, 2015

Member

What errors are they fixing?

Member

thatsIch commented May 9, 2015

What errors are they fixing?

@petabyteboy

This comment has been minimized.

Show comment
Hide comment
@petabyteboy

petabyteboy May 9, 2015

Contributor

Yup I was getting that too. There is an error because the resources are included twice in the Eclipse classpath.

Contributor

petabyteboy commented May 9, 2015

Yup I was getting that too. There is an error because the resources are included twice in the Eclipse classpath.

@thatsIch

This comment has been minimized.

Show comment
Hide comment
@thatsIch

thatsIch May 9, 2015

Member

What does eclipse consider a resource?

Member

thatsIch commented May 9, 2015

What does eclipse consider a resource?

@TheJulianJES

This comment has been minimized.

Show comment
Hide comment
Contributor

TheJulianJES commented May 9, 2015

This PR fixes: http://imgur.com/Fqltju1

@thatsIch

This comment has been minimized.

Show comment
Hide comment
@thatsIch

thatsIch May 9, 2015

Member

There are no resources? Tell eclipse its drunk!

Member

thatsIch commented May 9, 2015

There are no resources? Tell eclipse its drunk!

@thatsIch

This comment has been minimized.

Show comment
Hide comment
@thatsIch

thatsIch May 9, 2015

Member

nvm its probably about the Scanner usage.

The proper fix here is to use .close() and not hide the warning.

Member

thatsIch commented May 9, 2015

nvm its probably about the Scanner usage.

The proper fix here is to use .close() and not hide the warning.

@petabyteboy

This comment has been minimized.

Show comment
Hide comment
@petabyteboy

petabyteboy May 9, 2015

Contributor

My Eclipse even showed an error (not a warning). I can reproduce it if you want.

Contributor

petabyteboy commented May 9, 2015

My Eclipse even showed an error (not a warning). I can reproduce it if you want.

@TheJulianJES

This comment has been minimized.

Show comment
Hide comment
@TheJulianJES

TheJulianJES May 9, 2015

Contributor

@plumpudding Yeah, that link fixes the error. This PR fixes warnings I had in my two eclipse instances.

Contributor

TheJulianJES commented May 9, 2015

@plumpudding Yeah, that link fixes the error. This PR fixes warnings I had in my two eclipse instances.

@thatsIch

This comment has been minimized.

Show comment
Hide comment
@thatsIch

thatsIch May 9, 2015

Member

@TheJulianJES it does not fix them, you just hide them, which is bad. Warnings are a good thing, cause they can detect potential errors and bugs.

Member

thatsIch commented May 9, 2015

@TheJulianJES it does not fix them, you just hide them, which is bad. Warnings are a good thing, cause they can detect potential errors and bugs.

@TheJulianJES

This comment has been minimized.

Show comment
Hide comment
@TheJulianJES

TheJulianJES May 9, 2015

Contributor

@thatsIch I thought they are just issues in a eclipse (1. and 2.) and not in idea.

Contributor

TheJulianJES commented May 9, 2015

@thatsIch I thought they are just issues in a eclipse (1. and 2.) and not in idea.

@thatsIch

This comment has been minimized.

Show comment
Hide comment
@thatsIch

thatsIch May 9, 2015

Member

Generally its not bad, since this object will be cleaned up anyways after the code block is executed.
Also here it is applied to a String, so no external resources like files are in use, so it is actually eclipse messing up.

Member

thatsIch commented May 9, 2015

Generally its not bad, since this object will be cleaned up anyways after the code block is executed.
Also here it is applied to a String, so no external resources like files are in use, so it is actually eclipse messing up.

@TheJulianJES

This comment has been minimized.

Show comment
Hide comment
@TheJulianJES

TheJulianJES May 9, 2015

Contributor

Okay, now I'm messed up... So what is the problem currently?

Contributor

TheJulianJES commented May 9, 2015

Okay, now I'm messed up... So what is the problem currently?

@thatsIch

This comment has been minimized.

Show comment
Hide comment
@thatsIch

thatsIch May 9, 2015

Member

I accept the all fix, the resource is just wrong here, since there are no resources in the first place. But closing the Scanner is still a good thing, if you do not want to do it, I can do it.

Member

thatsIch commented May 9, 2015

I accept the all fix, the resource is just wrong here, since there are no resources in the first place. But closing the Scanner is still a good thing, if you do not want to do it, I can do it.

@TheJulianJES

This comment has been minimized.

Show comment
Hide comment
@TheJulianJES

TheJulianJES May 9, 2015

Contributor

@thatsIch Is it okay?

Contributor

TheJulianJES commented May 9, 2015

@thatsIch Is it okay?

@thatsIch

This comment has been minimized.

Show comment
Hide comment
@thatsIch

thatsIch May 9, 2015

Member

Make the Scanner final and you are done :)

Member

thatsIch commented May 9, 2015

Make the Scanner final and you are done :)

Fix warnings in eclipse
Close scanner instead of doing it with a warning

Make the scanner final..
@TheJulianJES

This comment has been minimized.

Show comment
Hide comment
@TheJulianJES

TheJulianJES May 9, 2015

Contributor

I usually make things never final..

Contributor

TheJulianJES commented May 9, 2015

I usually make things never final..

@thatsIch

This comment has been minimized.

Show comment
Hide comment
@thatsIch

thatsIch May 9, 2015

Member

Good practice, prevents you from modifying code without your consent

Member

thatsIch commented May 9, 2015

Good practice, prevents you from modifying code without your consent

thatsIch added a commit that referenced this pull request May 9, 2015

@thatsIch thatsIch merged commit 35997d5 into AppliedEnergistics:master May 9, 2015

1 check passed

default Finished TeamCity Build Applied Energistics :: Pull Requests : Tests passed: 35
Details
@TheJulianJES

This comment has been minimized.

Show comment
Hide comment
@TheJulianJES

TheJulianJES May 9, 2015

Contributor

@thatsIch Should I also pull this to rv2 or is it not necessary?

Contributor

TheJulianJES commented May 9, 2015

@thatsIch Should I also pull this to rv2 or is it not necessary?

@thatsIch

This comment has been minimized.

Show comment
Hide comment
@thatsIch

thatsIch May 9, 2015

Member

please do

Member

thatsIch commented May 9, 2015

please do

@TheJulianJES

This comment has been minimized.

Show comment
Hide comment
@TheJulianJES

TheJulianJES May 9, 2015

Contributor

Will do tomorrow

Contributor

TheJulianJES commented May 9, 2015

Will do tomorrow

@TheJulianJES TheJulianJES deleted the TheJulianJES:errorEclipse branch May 10, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment