Skip to content

Conversation

@lacostej
Copy link
Member

Pull Request Checklist

  • My pull request has been rebased on master
  • I ran bundle exec rspec to make sure that my PR didn't break any test
  • I ran bundle exec rubocop to make sure that my PR is inline with our code style
  • I have read the code of conduct

Pull Request Description

The documentation states that U3D_EXTRA_PATHS are windows style.

Fixes #383

@lacostej
Copy link
Member Author

lacostej commented Nov 21, 2019

I didn't test this :)

@lacostej lacostej force-pushed the fix/windows_extra_install_path branch from 9678e29 to 6f4dbed Compare November 21, 2019 13:09
@lacostej lacostej force-pushed the fix/windows_extra_install_path branch from 6f4dbed to 3be4934 Compare November 21, 2019 13:13
Copy link
Member

@niezbop niezbop left a comment

Choose a reason for hiding this comment

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

Not super found of that. I was thinking of tackling this as well. I would have tried the following:

  • File.expand_path(path) may actually work IIRC, I wanted to test this out. Plus it would be more Unix-friendly
  • Create a U3dCore::Helper method if expand_path doesn't work, such as the existing U3dCore::Helper.windows_path

@lacostej
Copy link
Member Author

👍 I let you test&fix this then.

@lacostej lacostej force-pushed the fix/windows_extra_install_path branch from 437c5d1 to 0fb0a8f Compare November 22, 2019 08:57
@lacostej
Copy link
Member Author

@niezbop

image

image

@niezbop
Copy link
Member

niezbop commented Nov 22, 2019

@lacostej Cool! I was pretty sure File.expand_path would work!

@niezbop
Copy link
Member

niezbop commented Nov 22, 2019

Not the platform you are looking for

💯

@lacostej lacostej force-pushed the fix/windows_extra_install_path branch from 3c45367 to c6318e7 Compare November 22, 2019 09:26
@lacostej lacostej force-pushed the fix/windows_extra_install_path branch from c6318e7 to a3e8704 Compare November 22, 2019 09:28
@lacostej lacostej requested a review from niezbop November 22, 2019 09:32
Copy link
Member

@niezbop niezbop left a comment

Choose a reason for hiding this comment

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

The implementation looks food to me, but I'd rather have a little cleanup of the specs.

@lacostej lacostej merged commit e980ddd into DragonBox:master Nov 22, 2019
@lacostej lacostej deleted the fix/windows_extra_install_path branch November 22, 2019 10:09
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.

U3D_EXTRA_PATHS is improperly interpreted on Windows

2 participants