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

sbt.bat: JAVACMD is not quoted in version check #228

Merged
merged 1 commit into from Apr 23, 2018

Conversation

Projects
None yet
3 participants
@jessicah
Copy link
Contributor

commented Apr 19, 2018

This fixes a failed version check for a path containing spaces.

sbt.bat: JAVACMD is not quoted in version check
This fixes a failed version check for a path containing spaces.

@eed3si9n eed3si9n added the ready label Apr 19, 2018

@dwijnand

This comment has been minimized.

Copy link
Member

commented Apr 20, 2018

I'm confused because I thought you don't quote things in bat files like in bash files to preserve spaces..

@jessicah

This comment has been minimized.

Copy link
Contributor Author

commented Apr 21, 2018

@dwijnand

This comment has been minimized.

Copy link
Member

commented Apr 22, 2018

one example that I'm remembering is this one, where removing quotes fixed an issue: 0e59118#diff-0e467aba72fad1d1010d8efae1c500d6

@jessicah

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2018

@jessicah

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2018

@dwijnand

This comment has been minimized.

Copy link
Member

commented Apr 23, 2018

I don't see where/how we're escaping a double quote with a double quote, but I see, as you said, the several pre-existing instances of double quoting _JAVACMD in the script.

thanks for bearing with me, @jessicah, sorry for the delay.

@dwijnand dwijnand merged commit c525dd1 into sbt:master Apr 23, 2018

1 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@dwijnand dwijnand removed the ready label Apr 23, 2018

@eed3si9n

This comment has been minimized.

Copy link
Member

commented Apr 23, 2018

This is a duplicate of #227, and as the AppVeyor failure indicates this doesn't completely fix all the problems.

@dwijnand

This comment has been minimized.

Copy link
Member

commented Apr 23, 2018

sorry.. I actually missing the .. wow

I've now protected the branch for Travis CI and AppVeyor checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.