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

Updating to modern Rust #84

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

umgefahren
Copy link

@umgefahren umgefahren commented Aug 13, 2023

This Pull Request brings the library into the standards and convention of Rust at the time of writing, at least from my point of view.

  • Everything that can be easily const is now const
  • Migration from unmaintained crate failure to thiserror
  • Migration from Rust Edition 2018 to 2021
    • Removal of the deprecated extern crate foo; syntax
    • Setting minimum Rust version to 1.56.0
    • Automatic migration by cargo fix
  • Implementation of latest suggestions (automatic and manual) by clippy. This introduces breaking changes, that's the reason for the major version bump.
  • Formatting
  • Encoded starting position using const.

I really like this innovative, unconventional use of build.rs, however it also greatly increases build time. It might be possible to replace the build.rs file entirely by clever use of const. It might be even easier, once more operations are available to const.

However the usage of static mut seems unnecessary, but I don't have the time to implement those changes.

On a additional note I think it's sufficient to rely on docs.rs.

src/bitboard.rs Outdated Show resolved Hide resolved
src/chess_move.rs Outdated Show resolved Hide resolved
@dmyTRUEk
Copy link

Overall, looks good to me, if only @jordanbray accepted it...

@yzoug
Copy link

yzoug commented Feb 17, 2024

This looks great and I would love for it to be merged.

However, having a PR (#71) open for almost 2 years now with no feedback, I don't think it ever will.

Would you guys be interested in forking this crate under a new name? There are a few open PRs that we could merge. I would be happy to help but I don't have much Rust experience.

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.

3 participants