Skip to content

Conversation

@m-sp
Copy link
Contributor

@m-sp m-sp commented Oct 7, 2017

No description provided.

Copy link
Member

@dottybot dottybot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Have an awesome day! ☀️

@allanrenucci allanrenucci self-assigned this Oct 9, 2017
@allanrenucci allanrenucci self-requested a review October 9, 2017 14:55
@allanrenucci allanrenucci removed their assignment Oct 9, 2017
Copy link
Contributor

@allanrenucci allanrenucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nitpicks. Otherwise LGTM

}.expect { (ictx, messages) =>
implicit val ctx: Context = ictx
assertMessageCount(1, messages)
val ExtendFinalClass(extendee, parent) :: Nil = messages
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably be extender and not extendee

val kind = "Syntax"
val msg = hl"$clazz cannot extend ${"final"} $finalClazz"
val explanation =
hl"""A class marked with the ${"final"} keyword cannot be extended by an object or a class"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would simply say "... cannot be extended". For instance, a trait cannot extends a final class either.

@m-sp
Copy link
Contributor Author

m-sp commented Oct 9, 2017

Thanks for the review, I've addressed your points.
I hope squashing is fine!

Copy link
Contributor

@allanrenucci allanrenucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you for your contribution ! 🎉

@allanrenucci allanrenucci merged commit 953a385 into scala:master Oct 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants