-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Script parser preserves target module #10563
base: 2.13.x
Are you sure you want to change the base?
Conversation
bbfb052
to
78926da
Compare
I tried reviewing this and now I can't stop yawning. |
Is the TODO in the PR description still a TODO? (And for this PR, or for some hypothetical future PR?) |
🥱 I believe that todo was aspirational, in the medical sense of
trying to snack while yawning uncontrollably. |
@som-snytt I'm going to go into schoolmarm mode here and say: I'm not sure I understand you, and also, this isn't only about it's about whether I personally understand. For the PR to be considered ready for review, can you edit the PR description to reflect the current state, as clearly as you possibly can? It's generally okay to be more freewheeling in our back-and-forth in the comments, but PR descriptions are documentation. |
Yes, I'll get back to this before 2.13.14! Edit: oh, I see I marked it ready for review. Maybe I wanted feedback on whether it's worth trying for 2.13.13, but it's not critical; it's just a nuisance when testing code snippets. |
Yeah, offhand I'd say this looks eventually-mergeable, but 2.13.14 would be fine. |
I needed this today. Ticket has
but |
Fixes scala/bug#12889
Allow adding main to an existing object.
It takes a prefix of top-level statements which remain top-level (which helps with any issues with local methods).
If the "target" module was already defined at the "bottom of the file", so that it winds up in the "main" method, then also synthesize a call to its existing main method or just initialize the lazy object if there is no main method. [TODO]