search

tov / succinct-rs

Succinct Data Structures for Rust
Apache License 2.0
56 stars 15 forks source link

cargo fmt and CI checks #7

Closed luizirber closed 5 years ago

luizirber commented 5 years ago

This modifies Travis CI to check for formatting, and also reformats the code using cargo fmt.

tov commented 5 years ago

Thanks! I’m not sure I want this, though—how useful is it to you? I don’t typically use rustfmt as part of my workflow, and I’m not actually sure how to add it so that my commits don’t fail CI all the time. Can you convince me it’s worthwhile? If you plan to contribute more, then making things nice for you would be a good reason to accept the change.

luizirber commented 5 years ago

Fair point, it is pretty aggressive to impose rustfmt on your workflow...

As for future contributions, the endgame here is fixing succinct-rs to run in wasm32 (since it's a transitive dependency for me, I'm using pdatastructs.rs). As the name implies, wasm32 is 32-bit and some implementations are missing (where checks for 64-bit pointer size were used, like here). I did a poor job in https://github.com/luizirber/succinct-rs/commit/5ee2c2a1503ff5d76edf3a77303054f4b1973284 to make it compile, but still need to check for correctness.

And, as you can see from the changes, it got reformatted with rustfmt because that's what my workflow does by default, and part of the reason I made this PR. But I can close this and clean up the wasm32 PR too.

UPDATE: the cleaned commit for wasm32 is https://github.com/luizirber/succinct-rs/commit/a0a69e2, I opened https://github.com/tov/succinct-rs/pull/9 for further discussion.