search

JuliaMath / Yeppp.jl

Yeppp! bindings
Other
55 stars 14 forks source link

Corrections for README.md #7

Closed hiccup7 closed 9 years ago

hiccup7 commented 9 years ago

1) For Windows, I found that I had to rename yeppp.dll from the zip distribution file to libyeppp.dll. Otherwise, Julia would not load it.

2) I put libyeppp.dll in Julia's bin folder, which contains the other dll files used by Julia, and this worked fine. I think this folder would be the best folder to suggest to new Windows users.

3) I think it would be helpful to new Windows users to know that Julia only needs the dll file, even though other files are also provided in the zip distribution under binaries.

4) README.md currently says that only Vector{Float64} arguments are supported, but the package now has more flexibility because it supports Array{Float64} arguments.

ViralBShah commented 9 years ago

Could you edit the README directly on github or provide a PR. Much appreciated.

kmsquire commented 9 years ago

A better fix would be to actually test for that library name, instead of asking the user to rename it. See, e.g., https://github.com/JuliaLang/GZip.jl/blob/master/src/zlib_h.jl#L3-L4 or https://github.com/quinnj/ODBC.jl/blob/master/src/ODBC_Types.jl#L2-L20.

hiccup7 commented 9 years ago

Doesn't the commit from an hour ago break OSX?

simonster commented 9 years ago

OS X counts as UNIX for the purpose of @unix_only.

hiccup7 commented 9 years ago

Would it be better to use the @windows ? syntax instead so that Android/ARM and future additions gets the same library name as linux & OSX? Maybe Android/ARM count as UNIX too, but someday there may be a new OS that neither falls under @unix_only nor @windows_only, so the UNIX naming is potentially the best guess at a default.

hiccup7 commented 9 years ago

I renamed my Windows dll back to the original name and tested the master branch. Works fine.

tkelman commented 9 years ago

The @windows? macros are implemented very hackily and likely to go away in the future.

For the best user experience, should probably set up BinDeps to download and extract the binaries to a package-private location and deal with the paths automatically.

hiccup7 commented 9 years ago

I edited README.md directly on github, which is my first ever github commit. Since I only have experience with Windows, I didn't specify the extracted file extensions needed on other OS. It may not be important if users extract an unnecessary file or two. The important thing to me was to specify the file name in Windows, which is different than the previous README documentation, to avoid confusion. This completes the PR for me.

@tkelman , how about a separate PR for your suggestion so it doesn't get lost?

ViralBShah commented 9 years ago

I don't see the PR.

ViralBShah commented 9 years ago

Never mind. I see it committed.