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

unconventional package objects #2781

Closed
kailuowang opened this issue Apr 9, 2019 · 2 comments · Fixed by #2785
Closed

unconventional package objects #2781

kailuowang opened this issue Apr 9, 2019 · 2 comments · Fixed by #2785

Comments

@kailuowang
Copy link
Contributor

we have several package objects that are not really package object since there is no corresponding packages and folders. Also their name is not conventional package.scala which throws warning in compiler.
e.g. kernel/instances/list, kernel/instances/tuple etc.
We can't make them normal objects now since that will break BC. But we should at least address the compiler warning and move those to package.scala and into corresponding folders.
e.g.
kernel/instances/list.scala -> kernel/instances/list/package.scala

@denisrosca
Copy link
Contributor

denisrosca commented Apr 10, 2019

There are a couple instances in there that are not marked as package objects (e.g. object eq extends EqInstances, finiteDuration doesn't seem to have an object at all).

Should they be left untouched or would you prefer moving them to package objects for consistency?

@kailuowang
Copy link
Contributor Author

Ieally all these objects should reside in a real package object like this
https://github.com/typelevel/cats/blob/master/core/src/main/scala/cats/instances/package.scala
I suspect that moving the non-package objects from their files into the real package object will not break binary compatibility, so, maybe worth a try and run mima on scala 2.11.12.
We don't need to create an object if it doesn't exist in the file, there should be an existing object in the real package object already.

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

Successfully merging a pull request may close this issue.

2 participants