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

SI-6162 Adds private[scala] @deprecatedInheritance/@deprecatedOverriding #1284

Merged
merged 4 commits into from Sep 11, 2012

Conversation

@retronym
Copy link
Member

@retronym retronym commented Sep 10, 2012

Second try submission of #1026

Review by @paulp, @soc

soc and others added 3 commits Jul 30, 2012
These annotations are meant to warn from inheriting a class or
from overriding a member, due to the reasons given in `msg`.

The naming and placement of the methods is in line with
@deprecated and @deprecatedName.
While they ought to be generalized to aribirary modifier
changes before being offered in the standard library, the
opportunity to use them in 2.10 is too important to pass up.

So for now, they're private[scala].

En route:

 - made the error messages more concise
 - fix positioning of inheritance error
 - improve test coverage
@retronym
Copy link
Member Author

@retronym retronym commented Sep 10, 2012

If this goes in, we should ask around in -internals for any other places we can slap these annotations. TupleN, perhaps?

@scala-jenkins
Copy link

@scala-jenkins scala-jenkins commented Sep 10, 2012

@scala-jenkins
Copy link

@scala-jenkins scala-jenkins commented Sep 10, 2012

Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1111/

@scala-jenkins
Copy link

@scala-jenkins scala-jenkins commented Sep 10, 2012

jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1111/

@scala-jenkins
Copy link

@scala-jenkins scala-jenkins commented Sep 10, 2012

@jsuereth
Copy link
Member

@jsuereth jsuereth commented Sep 11, 2012

This lgtm

@pavelpavlov

This comment has been minimized.

You mean [[scala.deprecatedOverriding]] here?

This comment has been minimized.

Copy link
Owner Author

@retronym retronym replied Sep 11, 2012

Thanks, fixed.

@gkossakowski
Copy link
Member

@gkossakowski gkossakowski commented Sep 11, 2012

LGTM from me as well. Can we squash pull request feedback commit?

@retronym
Copy link
Member Author

@retronym retronym commented Sep 11, 2012

I can't squash it right now, but feel free to do so when merging. Personally, I like leaving a single "pull request feedback" commit (maybe itself squashed with a few iterations of feedback.) in the history, so long as the previous commits are not broken.

@gkossakowski
Copy link
Member

@gkossakowski gkossakowski commented Sep 11, 2012

I agree that leaving them is fine but I'd prefer to have a bit better commit message than just Pull request feedback.

This commit is trivial so Pull request #1284 feedback. Those numbers turn to be useful for grepping and general scanning through commit.

For this one I'll go ahead and merge.

gkossakowski added a commit that referenced this pull request Sep 11, 2012
SI-6162 Adds private[scala] @deprecatedInheritance/@deprecatedOverriding
@gkossakowski gkossakowski merged commit 4f696fe into scala:2.10.x Sep 11, 2012
@retronym
Copy link
Member Author

@retronym retronym commented Sep 11, 2012

BTW, if you want to answer the question: "what pull request contained this commit", you can configure git to fetch all the pull requests and then use the usual git commands, e.g git branch --contains <hash>

@gkossakowski
Copy link
Member

@gkossakowski gkossakowski commented Sep 11, 2012

Ah yes. Thanks for good suggestion!

@bajohns

This comment has been minimized.

Copy link

@bajohns bajohns commented on c78fe02 Nov 25, 2012

Looks like there is a spelling mistake here. /me/be/

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