Skip to content
This repository has been archived by the owner on Dec 22, 2021. It is now read-only.

Don’t replace Stream with LazyList #528

Closed
wants to merge 2 commits into from

Conversation

julienrf
Copy link
Contributor

@julienrf julienrf commented Mar 28, 2018

Because their behaviour is not the same (LazyList has a lazy head) I think we should not rewrite Stream usage to LazyList. Note that Stream is deprecated in 2.13, though, that’s why I’m not totally sure what should be the right decision.

(this PR depends on #527)

@szeiger
Copy link
Member

szeiger commented Apr 3, 2018

Can we modularize these rules? It would be nice if you could perform this migration separate from other ones. In most cases you'll want to use LazyList and do not need any additional changes to make it work.

@julienrf
Copy link
Contributor Author

julienrf commented Apr 3, 2018

What we could do:

  • provide each rule separately, so that users can have fine grained control on which ones to apply,
  • provide a “pack” of rules (ie. a single rule that aggregates several rules) that migrate a 2.12 code base to a 2.13 code base,
  • provide a “pack” of rules that migrates a 2.12 code base to a cross-compatible 2.12/2.13 code base (using the collections-compat library).

@julienrf
Copy link
Contributor Author

I’m closing this PR. We should still provide this rule and let users not apply it if they don’t want to. I’ve created #559 to keep track of that.

@julienrf julienrf closed this Apr 18, 2018
@SethTisue
Copy link
Member

I hadn't commented because I wasn't sure what I thought about this. But I'm now comfortable with this outcome. 👍

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

Successfully merging this pull request may close these issues.

None yet

3 participants