savage13 / miniseed-rs

MiniSEED library for rust
MIT License
4 stars 3 forks source link

Rename to libmseed-sys #2

Open HerrMuellerluedenscheid opened 3 years ago

HerrMuellerluedenscheid commented 3 years ago

Hi!

AFAIK the common naming for ffi binding libraries would be lib...-sys. Thus, how about renaming this crate to libmseed-sys? Best Marius

savage13 commented 3 years ago

I think it might be best to create a new repo with the new name libmseed-sys for the 3.x version of the code. I can mark this version as only being able to use only the 2.x C version of libmseed.

I would also like to hear what @acarapetis thinks as well.

Thoughts ?

This library is not really a pure FFI, although it should have been (you live and learn).

HerrMuellerluedenscheid commented 3 years ago

Sounds good to me 👍 Might also avoid some confusion. @acarapetis what do you think?

HerrMuellerluedenscheid commented 3 years ago

Or actually I'm more in favor of renaming the repo. If there are two repos with similar purposes out there might actually cause more confusion. Better to have one single source of truth and one go-to repo, I think.

acarapetis commented 3 years ago

I think libmseed-sys is the perfect name for an FFI-only crate, but I'd love to keep a more friendly interface available too! I'm new to the rust community, but based on what I've seen, the ideal setup would something like this:

I don't think this should be confusing so long as each library explains its relation to the other somewhere early on in the README.

I would hope that adoption of this library is rare enough that we could easily get away with making changes this big. Renaming the wrapper crate from miniseed-rs to miniseed might be an idea - including the -rs in a crate name seems like an anti-pattern to me (though it makes sense to keep it in the github repository name). This would avoid breaking users of the existing miniseed-rs crate, though it would leave that deprecated crate hanging around unmaintained.

Thoughts? I'd be happy to take on maintaining the wrapper library if you want.

acarapetis commented 3 years ago

(Also, if we're breaking things and separating the bindings from the wrapper, we should probably rename ms_record, ms_input, etc. to use the standard CamelCase.)

acarapetis commented 3 years ago

Oh, and while I remember, another tip I came across while reading https://kornel.ski/rust-sys-crate was to avoid cloning the libmseed source in the build phase, and instead just bundle the source in the crate. This makes a lot of sense to me - just pin the version of libmseed we've tested with as a submodule, and we can avoid depending on the network at build time.

HerrMuellerluedenscheid commented 3 years ago

@acarapetis yes I think that is the recommended way to have an ffi libmseed-sys and miniseed in two different libraries. Shall we open an organization or github team for that to host both. That would also nicely indicate that both belong together.

savage13 commented 3 years ago

I created libmseed-sys for the 3.x version of libmssed. See https://github.com/savage13/libmseed-sys . I am creating and populating a initial repo of miniseed (https://github.com/savage13/miniseed).

Nothing has been published to crates.io.

This crate (Version 2.x) has the name "miniseed" according to crates.io. If we would like to keep the crate name as miniseed, we would either need to either

HerrMuellerluedenscheid commented 3 years ago

@savage13 would you be open for the idea to create a github group and add @acarapetis and me as maintainers? I think that will help to organize things quite nicely. I can imagine that that would be also a nice place for further crates for seismological data processing using rust 👍

savage13 commented 3 years ago

@savage13 would you be open for the idea to create a github group and add @acarapetis and me as maintainers?

Yes

acarapetis commented 3 years ago

I think a seismo rust group makes a lot of sense :)