-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Port generateSources
to Scala 3 Library
#23705
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
generateSources
to DottygenerateSources
to Scala 3 Library
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.
I left a few inline comments. Most of those relate to code style or comment consistency but I have some concerns about the logic of the function converters, which I found particularly difficult to follow.
def numeric = List(B, S, C, I, L, F, D) | ||
def values = List(U, Z) ++ numeric |
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.
def numeric = List(B, S, C, I, L, F, D) | |
def values = List(U, Z) ++ numeric | |
def numeric = List(B, S, C, I, L, F, D) | |
def values = List(U, Z) ++ numeric |
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.
Looks okay to me.
I think there is still a ton of inconsistent styling and dubious whitespaces but perhaps it is not worth dwelling on it. It would be nice if we could apply some automatic guidelines to address those 😔
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.
I didn't want to port those generators as we don't change these files much and it will just make the build more complex. I'm against merging this for now as the added complexity in the build is not worth the benefits. Also, these files are fundamental to Scala 3 (and Scala 2) so if we were to modify them, it would be best to change them in Scala 2 and have them in Scala 3 when synchronising the files (unless the change is not compatible with Scala 2).
@natsukagami can we close this one too? |
The generators used in
generateSources
are copied over in the first commit, and changes to make them match the current source code is presented in the second commit.This includes adding
import scala.language.`2.13`
into the generated sources.It is added as a
generateSources
command insidescala-library-(non)boostrapped
projects.Note that the
genprod
file was renamed toGenerateLibraryNTemplates
, but I still think the name is somewhat unclear. Suggestions welcomed.