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

Fix #4499: Shorten internal module IDs in FewestModules mode #4501

Merged
merged 1 commit into from Jun 7, 2021

Conversation

gzm0
Copy link
Contributor

@gzm0 gzm0 commented Jun 5, 2021

No description provided.

@gzm0 gzm0 force-pushed the shorter-internal-modules branch from 12bc79b to dcaf6fe Compare June 5, 2021 19:28
@gzm0 gzm0 requested a review from sjrd June 5, 2021 19:28
@gzm0 gzm0 linked an issue Jun 5, 2021 that may be closed by this pull request
@gzm0 gzm0 force-pushed the shorter-internal-modules branch from dcaf6fe to 5dd5e5e Compare June 6, 2021 10:58
Copy link
Member

@sjrd sjrd left a comment

Choose a reason for hiding this comment

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

Only a few indentation issues. Otherwise LGTM.

Comment on lines 36 to 40
val classDefs = Seq(
mainTestClassDef({
consoleLog(str("Hello World!"))
})
)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
val classDefs = Seq(
mainTestClassDef({
consoleLog(str("Hello World!"))
})
)
val classDefs = Seq(
mainTestClassDef({
consoleLog(str("Hello World!"))
})
)

Comment on lines 42 to 46
val expectedFiles = Set(
"internal-0.js", // public module
"internal-1.js", // public module
"internal-2.js" // internal module, avoiding internal-0 and internal-1.
)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
val expectedFiles = Set(
"internal-0.js", // public module
"internal-1.js", // public module
"internal-2.js" // internal module, avoiding internal-0 and internal-1.
)
val expectedFiles = Set(
"internal-0.js", // public module
"internal-1.js", // public module
"internal-2.js" // internal module, avoiding internal-0 and internal-1.
)

@gzm0 gzm0 force-pushed the shorter-internal-modules branch from 5dd5e5e to 15026b2 Compare June 7, 2021 16:03
@gzm0
Copy link
Contributor Author

gzm0 commented Jun 7, 2021

Updated.

@sjrd sjrd merged commit ee48b8d into scala-js:master Jun 7, 2021
@gzm0 gzm0 deleted the shorter-internal-modules branch June 12, 2021 11:20
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.

FewestModules can result in filenames that are greater than 255 Characters
2 participants