Skip to content

Conversation

@indexzero
Copy link

I've seen this module be enough of a problem on Windows that I feel we need to either:

  1. Fix it so that it works in a cross-platform way
  2. Disable it on Windows and fail gracefully.

This pull-request does (2).

@3rd-Eden
Copy link
Member

@indexzero: It would be nice know what the actual problems are before I disable it completely. As most of these bugs can probably be fixed relatively easily

@indexzero
Copy link
Author

@3rd-Eden bash does not exist on Windows: https://github.com/observing/pre-commit/blob/master/hook

@3rd-Eden
Copy link
Member

@indexzero it's easy enough to replace that with a bat file during the installation phase. https://github.com/observing/pre-commit/blob/master/install.js#L20

@indexzero
Copy link
Author

@3rd-Eden absolutely. I know nothing about batch scripts so I thought disabling it for now was the best bet.

@Swaagie
Copy link
Member

Swaagie commented Jul 30, 2015

There are some more issue on windows in general though, might be good to fix those all at once ;)

@3rd-Eden
Copy link
Member

@Swaagie so you're suggesting installing linux when installing this module ;-)?

@Swaagie
Copy link
Member

Swaagie commented Jul 30, 2015

haha

@3rd-Eden
Copy link
Member

Fixed in #41 with a more graceful solution.

@3rd-Eden 3rd-Eden closed this Jul 30, 2015
@indexzero indexzero deleted the win32 branch July 31, 2015 00:49
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.

3 participants