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

Drop compat code for java 8 #2155

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Drop compat code for java 8 #2155

wants to merge 1 commit into from

Conversation

pm47
Copy link
Member

@pm47 pm47 commented Jan 27, 2022

It has been a long time and officially supported version is Java 11.

This can wait after the upcoming release though.

@pm47 pm47 requested a review from t-bast January 27, 2022 15:17
t-bast
t-bast previously approved these changes Jan 27, 2022
Copy link
Member

@t-bast t-bast left a comment

Choose a reason for hiding this comment

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

Let's get this in now, we could have gotten rid of this a while ago already

@pm47
Copy link
Member Author

pm47 commented Jan 27, 2022

There is actually a catch, as c848cfe shows. getSimpleClassName actually returns the full or simple class name depending on whether it is applied to an object or a class!

@pm47
Copy link
Member Author

pm47 commented Jan 27, 2022

I'm interested in your opinion on obj.getClass.getPrettySimpleName vs obj.prettyClassName.

This is most consistent with Java API, it adds
`obj.getClass.getPrettySimpleName` to the existing
`obj.getClass.get*Name`.

Instead we could go with a nicer `obj.prettyClassName`.
@t-bast
Copy link
Member

t-bast commented Jan 27, 2022

Since this isn't trivial after all, I think it should wait for after the release.

Foo.Bar -> "Bar",
Foo.Baz(42) -> "Baz"
).foreach { case (o, ref) => assert(o.getClass.getPrettySimpleName == ref)
}
Copy link
Member

Choose a reason for hiding this comment

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

nit: indent is weird

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.

None yet

2 participants