Skip to content

Two additions to batch files #13790

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

Merged
merged 1 commit into from
Oct 22, 2021
Merged

Conversation

michelou
Copy link
Contributor

These additions are follow-ups of the two merged pull requests :

The changes are Windows-specific and have no impact on the CI build.

Copy link
Contributor

@BarkingBad BarkingBad left a comment

Choose a reason for hiding this comment

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

LGTM

@BarkingBad
Copy link
Contributor

@philwalk would you also take a look? I know you test a lot on Windows

@anatoliykmetyuk anatoliykmetyuk merged commit 78824ad into scala:master Oct 22, 2021
@michelou michelou deleted the dotty-test-mrJar branch October 22, 2021 19:30
@michelou
Copy link
Contributor Author

michelou commented Nov 2, 2021

@smarter Backport candidate to 3.1.1-RC2.

@smarter
Copy link
Member

smarter commented Nov 2, 2021

Does this fix a regression from a previous scala 3 release? Otherwise it's unlikely to be backported.

@michelou
Copy link
Contributor Author

michelou commented Nov 2, 2021

@smarter My reasoning is as follows :

In other words the feature "batch files support installation path containing spaces"
(Reminder: Windows specific and no impact on the CI build)

  • was introduced in 3.1.0 (see "Other bug fixes" in announcement)
  • would work as expected in upcoming 3.1.1.

The final decision is yours.

@smarter
Copy link
Member

smarter commented Nov 3, 2021

I'm not familiar with this code so I shouldn't decide really :). You can submit a backport and have @anatoliykmetyuk decide since he reviewed this PR already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants