search

JuliaIO / GZip.jl

A Julia interface for gzip functions in zlib
https://juliaio.github.io/GZip.jl/dev
MIT License
39 stars 30 forks source link

Exhaustive operating system check in tests #56

Closed ararslan closed 8 years ago

ararslan commented 8 years ago

Ref https://github.com/JuliaIO/GZip.jl/pull/55#discussion_r70733030. cc @kmsquire

kmsquire commented 8 years ago

Sorry, but could you change this to throw(ErrorException(...))? This has been the direction in Base, and I, for one, am hoping to eventually reclaim error for logging purposes.

ararslan commented 8 years ago

Sure thing!

ararslan commented 8 years ago

AppVeyor wasn't passing before and the current failures aren't related to this. The OS X failure is a stalled build; the rest of the OS X and Linux builds passed.

tkelman commented 8 years ago

Well ErrorException isn't usually what you'd throw, since that has the same result as error. It's mainly useful to throw a specific exception type if users may need to catch and handle the error, which I doubt would be the case here.

I'd be amazed if Julia could get as far as trying to load this package on something that was neither windows or unix.

kmsquire commented 8 years ago

I wasn't really concerned about the error type as much as using the error function.

As I mentioned in my original comment, it doesn't matter to me if we have 2 cases (Windows/other) or 3 cases (Windows/unix/other).

@tkelman, your comment suggested a preference, but was somewhat noncommittal. Would you prefer this changed, or is it ok to merge as is?

tkelman commented 8 years ago

hm, ok, I don't see the difference given the definition of error. There are a few cases where "other" means an error in julia itself, but it's not entirely consistent, many other places assume one of the two will always be the case. I think it was fine as is, but merging this doesn't hurt anything either.

ararslan commented 8 years ago

I'm going to go ahead and close this for now but if someone is really into this idea then they can reopen, restore, merge, or whatever.