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

Backport LazyList #290

Merged
merged 3 commits into from
Feb 26, 2020
Merged

Backport LazyList #290

merged 3 commits into from
Feb 26, 2020

Conversation

NthPortal
Copy link
Contributor

@NthPortal NthPortal commented Jan 11, 2020

Still TODO:

  • Fix compilation on 2.11
  • Fix serialization

@NthPortal NthPortal added the library Related to compat library code (not rewrite rules) label Jan 11, 2020
@NthPortal NthPortal marked this pull request as ready for review January 12, 2020 10:38
@NthPortal NthPortal changed the title Backport LazyList [WIP] Backport LazyList Jan 12, 2020
@NthPortal
Copy link
Contributor Author

NthPortal commented Jan 12, 2020

I wish I could mark this as a draft again, cuz boy is it not done.
help welcome

@NthPortal NthPortal added the help wanted Extra attention is needed label Jan 12, 2020
@julienrf
Copy link
Contributor

@NthPortal Could you explain what remains to be done?

@NthPortal
Copy link
Contributor Author

@julienrf port GC tests, and make all the tests pass in all versions (or comment out/fix those that can't due to compiler bugs that weren't fixed until 2.13)

@NthPortal
Copy link
Contributor Author

NthPortal commented Jan 19, 2020

if someone can take a look at the last two little bits and make them compile, that would be hugely appreciated (TODO list in description)

@NthPortal NthPortal changed the title [WIP] Backport LazyList Backport LazyList Jan 19, 2020
@NthPortal NthPortal removed the help wanted Extra attention is needed label Feb 5, 2020
@NthPortal
Copy link
Contributor Author

I've given up on getting the serialization test to work across versions. it will just have to be specific to certain versions (2.11 and 2.13 JVM)

@NthPortal
Copy link
Contributor Author

it seems GC tests don't work on JS (idk why I missed this before). Is there a good way to let them run only on JVM? or can they be tweaked to use something other than Thread.sleep?

@NthPortal
Copy link
Contributor Author

NthPortal commented Feb 5, 2020

The docs for System.gc() say that when the method returns, the GC will already have been run (to the extent the "best effort" could), so it's not clear to me that the Thread.sleep(10) is necessary at all.

However, even without the Thread.sleep, the tests still fail for scala.js (probably because System.gc() is a NOP in scala.js). While JS engines do tend to have garbage collectors, there doesn't appear to be a way to manually invoke them (which is fair, from a browser design perspective).

I'm not sure what the best path forward is here, but if someone has one, they can feel free to (force) push it to this branch.

@yannbolliger
Copy link
Contributor

The fix for this is here: NthPortal#1

@julienrf julienrf merged commit 6822f09 into scala:master Feb 26, 2020
@julienrf
Copy link
Contributor

Thank you both!

@NthPortal NthPortal deleted the topic/lazylist/PR branch February 27, 2020 23:06
@nilskp
Copy link

nilskp commented Apr 6, 2020

Do we know when this will be released?

@julienrf
Copy link
Contributor

julienrf commented Apr 6, 2020

Hey, we should indeed cut a release soon, thanks for asking 😃
I think it would be good to have Lukas’ work in it. Let’s wait for a couple of weeks to see if we manage to merge it.

@lrytz
Copy link
Member

lrytz commented Apr 7, 2020

Is there a reason not to release often? It's little work, we have everything automated, no?

@julienrf
Copy link
Contributor

julienrf commented Apr 7, 2020

I think last time we had an issue and had to manually publish things. But I agree that in principle we could release more often.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
library Related to compat library code (not rewrite rules)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants