twitchtv / retool

Vendoring for executables written in Go
Apache License 2.0
417 stars 21 forks source link

Retool doesn't work on windows #23

Closed ghost closed 7 years ago

ghost commented 7 years ago

Currently retool doesn't work on windows. This is due to sync.go. The sync function uses exec.Command to delete a directory by using rm which is a linux specific command.

https://github.com/twitchtv/retool/blob/master/sync.go#L11

I have a fix+test, but am waiting on approval from my supervisor to open up the PR.

spenczar commented 7 years ago

Nice catch, thank you very much. I'm looking forward to the PR! I hope we can get a testcase in for this too.

Have you encountered other problems? Retool hasn't had much windows use, but I think it's very, very important and want to get it right.

On Fri, Mar 31, 2017 at 6:24 PM JohnRowleySEL notifications@github.com wrote:

Currently retool doesn't work on windows. This is due to sync.go. The sync function uses exec.Command to delete a directory by using rm which is a linux specific command.

https://github.com/twitchtv/retool/blob/master/sync.go#L11

I have a fix+test, but am waiting on approval from my supervisor to open up the PR.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/twitchtv/retool/issues/23, or mute the thread https://github.com/notifications/unsubscribe-auth/AA1vHRbWNWWCK_40XD2L0UZgjL49s9Lwks5rrX0MgaJpZM4MwOFV .

ghost commented 7 years ago

I haven't encountered any other issues so far, but I will be sure to let you know if I encounter any. I am personally not a windows user, but some of the devs on my team are.

still waiting for approval ( I have finished the code + test case though )

spenczar commented 7 years ago

@JohnRowleySEL Any luck on approval? I would love your name in the contributors list, but if it's too hard, I can write the code and test too, just to get Windows support out for everybody.

ghost commented 7 years ago

@spenczar Actually, I just got approval this morning!

spenczar commented 7 years ago

Fixed in #24

spenczar commented 7 years ago

Thanks for the contribution! I've tagged and release v1.3.1 which includes your fix.