Skip to content
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

clean out of warnings ;) #18

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

clean out of warnings ;) #18

wants to merge 4 commits into from

Conversation

rakion99
Copy link

No description provided.

rakion99 added 4 commits February 27, 2017 17:54
new parameter: boolean itf if the method's owner class is an interface.
new parameter: boolean itf if the method's owner class is an interface.
added missing argument to fix deprecated warning
added missing argument to fix deprecated warning
@LexManos
Copy link
Contributor

IIRC the reason this wasn't done is because it breaks backwards compatibility with older ASM libraries for no particular reason. Yes we compile against 5+ but IIRC we can run with 4+ for people who don't care about J8

@rakion99
Copy link
Author

oh ya i forgot about people still using J7

@Pokechu22
Copy link

Is that still a problem now that Minecraft 1.12 requires Java 8?

@jamierocks
Copy link

You forget that LaunchWrapper is not tied to the latest version of Minecraft, and others may want to mod older versions of Minecraft, using latest LaunchWrapper, without J8 (I can't say why they'd want to, but I guess that's why).

@Pokechu22
Copy link

Hm, yeah, I don't exactly know the version scheme that's used for LaunchWrapper. I don't personally see the issue with just saying "all versions after this one will require Java 8", but I don't know how often the version is bumped for this project either.

@LexManos
Copy link
Contributor

LexManos commented Jan 2, 2018

Honestly, I would only ever condone bumping required java version for a legitimate functional reason.
Cleaning some dev time warnings is not a legitimate functional reason. So it'll have to be justified a different way.
If people care about them they can toss a @SuppressWarnings on it.

Copy link

@Hallowizer Hallowizer left a comment

Choose a reason for hiding this comment

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

These changes look great!

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.

5 participants