-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix @experimental @main methods #13858
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change looks good but I think it'd make sense to keep all annotations instead of special-casing just this one for simplicity of specification.
41c08ba
to
5ea523e
Compare
I am not sure about keeping all annotations. Some method annotations don't even make sense for classes. For now I'd keep it to @experimental. If we get customizable entry methods, we could define a meta annotation |
OK, I think that's fine too and we can adjust based on usecases. |
These annotations are added to the For example with @main @foo @bar def entry() = ... we generate class foo:
<java static> @foo @bar def main(args: Array[String]) = ... entry() ... |
Ah, OK. That makes more sense then. Yes, maybe we should copy all
annotations except @main itself.
…On Wed, Nov 3, 2021 at 10:38 AM Nicolas Stucki ***@***.***> wrote:
Some method annotations don't even make sense for classes.
These annotations are added to the def main(args: Array[String]) method
and not to the class that contains that method.
For example with
@main @foo @bar def entry() = ...
we generate
class foo:
<java static> @foo @bar def main(args: Array[String]) = ... entry() ...
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#13858 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAGCKVR5HJZ2GJEQCYB6CJ3UKD7JLANCNFSM5HEJBRPA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
--
Martin Odersky
Professor, Programming Methods Group (LAMP)
Faculty IC, EPFL
Station 14, Lausanne, Switzerland
|
5ea523e
to
2e7d6af
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise LGTM.
a46e0dc
to
55dcf5d
Compare
Co-authored-by: Guillaume Martres <[email protected]>
55dcf5d
to
8fafdad
Compare
@timotheeandres you will have to rebase on master once this branch is merged. |
Fixes #13848