Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support lib #6

Merged
merged 2 commits into from
Nov 27, 2018
Merged

support lib #6

merged 2 commits into from
Nov 27, 2018

Conversation

jacobrosenthal
Copy link
Contributor

Are you open to library support?

For my svd2rust needs Id like to use as a library if only because I can do proper versioning in the cargo manifests vs binaries which seems not ready yet.

There seems to be several ways to support both main and lib. I'm very new to rust so I wouldn't be surprised if I didn't land on the most idiomatic way and as such I'm very open to suggestions here.

Also rustfmt wanted to have a go, so I put that in a separate commit. I have no opinons there.

@djmcgill
Copy link
Owner

I'm certainly open to exposing a library interface too, I'm not sure I understand your use case though - you can download specific versions of a binary with, e.g., cargo install form-0.3.0 and also I wouldn't expect this binary to be used as part of a build since the files involved are very static. Do you check in your generated sources or do something like rerunning svd2rust in your builds?

@jacobrosenthal
Copy link
Contributor Author

Thanks for the interest.

Im looking at a lot of crates and finding bash files installing binaries at versions as you said.
https://github.com/atsamd-rs/atsamd/blob/master/update.sh

But Id like to update the svd locally on my machine and not in a container and not pollute my environment. Also bash files are not cross platform, though I suppose Ive seen cargo make solutions that could work.

In my mind binaries in the future would allow local install like npm especially around dev dependencies which can then be properly tracked in the cargo manifest. I think Ive seen some open issues around this idea being worked on.

In this case Im just getting some rust practice, merging svd2rust, form, and rustfmt into an aliased command
https://github.com/jacobrosenthal/efm32-rs/blob/upstream/tools/src/bin/gen.rs

@djmcgill
Copy link
Owner

djmcgill commented Nov 27, 2018

Okay, I'm happy to merge this for now but if this use takes off I certainly would want to refine this crate more - some actual thought into the API exposed, more options, unit tests, integration tests, a CI setup etc.

Luckily we're still in pre-1.0.0 so are free to make these breaking changes.

edit: actually I've just seen that in your usage you have direct access to the code AST, so we could have a method that accepts that instead of serializing and re-parsing via a String: https://github.com/jacobrosenthal/efm32-rs/blob/master/tools/src/bin/gen.rs#L32

@djmcgill djmcgill merged commit 04af5b2 into djmcgill:master Nov 27, 2018
@djmcgill
Copy link
Owner

I'll publish this as 4.1.0 if you agree since it doesn't change existing behavior for the current users of the binary.

@jacobrosenthal
Copy link
Contributor Author

jacobrosenthal commented Nov 27, 2018

Thanks! Truly appreciate your time for the original code and continued maintenance :)

@jacobrosenthal jacobrosenthal mentioned this pull request Dec 14, 2018
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.

2 participants