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

SI-8339 remove deprecated rewrite of withFilter -> filter #5252

Merged
merged 1 commit into from Aug 12, 2016

Conversation

Projects
None yet
3 participants
@adriaanm
Member

adriaanm commented Jun 28, 2016

Let's try this again. Cross-ref #3566

@scala-jenkins scala-jenkins added this to the 2.12.0-RC1 milestone Jun 28, 2016

@szeiger

This comment has been minimized.

Show comment
Hide comment
@szeiger

szeiger Aug 3, 2016

Contributor

Did you run a community build for this PR?

Contributor

szeiger commented Aug 3, 2016

Did you run a community build for this PR?

@adriaanm

This comment has been minimized.

Show comment
Hide comment
@adriaanm

adriaanm Aug 3, 2016

Member

Not yet.

Member

adriaanm commented Aug 3, 2016

Not yet.

@adriaanm adriaanm added the on-hold label Aug 3, 2016

@adriaanm

This comment has been minimized.

Show comment
Hide comment
@adriaanm

adriaanm Aug 11, 2016

Member

(rebased to latest 2.12.x to see if that fixes the community build fail?)

Member

adriaanm commented Aug 11, 2016

(rebased to latest 2.12.x to see if that fixes the community build fail?)

@adriaanm adriaanm removed the on-hold label Aug 11, 2016

@adriaanm

This comment has been minimized.

Show comment
Hide comment
@adriaanm

adriaanm Aug 11, 2016

Member

community build is green

Member

adriaanm commented Aug 11, 2016

community build is green

@szeiger

This comment has been minimized.

Show comment
Hide comment
@szeiger

szeiger Aug 11, 2016

Contributor

nme.filter is also referenced in RefChecks and ReificationSupport. It looks to me like these special cases can also be removed.

Contributor

szeiger commented Aug 11, 2016

nme.filter is also referenced in RefChecks and ReificationSupport. It looks to me like these special cases can also be removed.

SI-8339 drop deprecated fallback `withFilter` -> `filter`
You must implement the `withFilter` method to use
`if`-guards in a `for`-comprehension.

(Drop pos/t7239.scala because it relied on this rewrite.)
@adriaanm

This comment has been minimized.

Show comment
Hide comment
@adriaanm

adriaanm Aug 11, 2016

Member

I removed the one from refchecks, but settled for a TODO in ReificationSupport as I didn't have time to dig into what this is meant to do.

Member

adriaanm commented Aug 11, 2016

I removed the one from refchecks, but settled for a TODO in ReificationSupport as I didn't have time to dig into what this is meant to do.

@szeiger

This comment has been minimized.

Show comment
Hide comment
@szeiger

szeiger Aug 12, 2016

Contributor

LGTM

Contributor

szeiger commented Aug 12, 2016

LGTM

@szeiger szeiger merged commit 1950412 into scala:2.12.x Aug 12, 2016

5 checks passed

cla @adriaanm signed the Scala CLA. Thanks!
Details
integrate-ide [2834] SUCCESS. Took 8 s.
Details
validate-main [3192] SUCCESS. Took 125 min.
Details
validate-publish-core [3133] SUCCESS. Took 28 min.
Details
validate-test [2762] SUCCESS. Took 81 min.
Details

@adriaanm adriaanm added the 2.12 label Oct 29, 2016

Blaisorblade added a commit to dotty-staging/dotty that referenced this pull request May 30, 2018

Stop rewriting withFilter to filter
This was dropped for Scala 2 in scala/scala#5252 for
2.12.0-RC1, as discussed for instance in
scala/bug#6455 and
scala/bug#8339.

Blaisorblade added a commit to dotty-staging/dotty that referenced this pull request May 30, 2018

Stop rewriting withFilter to filter
This was dropped for Scala 2 in scala/scala#5252 for 2.12.0-RC1, as discussed
for instance in scala/bug#6455 and scala/bug#8339.

Beyond this code, also remove `MaybeFilter` attachment and attending code
introduced in fcea3d5.

A caveat is that, unexpectedly, this code was added in 2016-08-24, after
scala/scala#5252 was merged, as if the decision was reversed; but lampepfl#1461 says:

> Also, I went through tests in pending/pos and reclassified as far as possible.

So I assume this just happened because the Scala 2 PR didn't remove our pending
test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment