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

chore: bump proc-macro-crate to 2.0.0 in borsh-derive #256

Merged
merged 2 commits into from
Nov 8, 2023
Merged

chore: bump proc-macro-crate to 2.0.0 in borsh-derive #256

merged 2 commits into from
Nov 8, 2023

Conversation

serprex
Copy link
Contributor

@serprex serprex commented Nov 6, 2023

Raise MSRV to 1.67

Enables getting latest toml crates in dependencies


Edit: upgrading proc-macro-crate is useful with respect to supporting borsh package rename in workspace level
This is illustrated by sample repo

@serprex serprex requested review from frol and dj8yfo as code owners November 6, 2023 15:08
@dj8yfo
Copy link
Collaborator

dj8yfo commented Nov 7, 2023

@serprex please fix CI.
Or otherwise attach a short proof that minimal rust version has to be increased from 1.66 in order to support this change.

Please also provide a short memo in description, what the advantages for borsh lib of bumping proc-macro-crate from 1 to 2 are.

@serprex
Copy link
Contributor Author

serprex commented Nov 7, 2023

proc-macro-crate 2 MSRV is 1.66 according to their small version 2 release: bkchr/proc-macro-crate@39a7c18

I noticed rust_decimal pulling in old toml crate versions due to this

But, you're right & toml_datetime 0.6.5 raised their MSRV to 1.67

So I've fixed PR with MSRV being raised to 1.67, but understand rejecting this PR since my original assessment didn't involve raising MSRV

@dj8yfo dj8yfo changed the title derive: proc-macro-crate 2 chore: bump proc-macro-crate to 2.0.0 in borsh-derive Nov 7, 2023
@dj8yfo
Copy link
Collaborator

dj8yfo commented Nov 7, 2023

@frol please take a look. I'll revert second commit if it's ok to bump msrv to 1.67

README.md Outdated
[borsh: rustc 1.66+]: https://img.shields.io/badge/rustc-1.66+-lightgray.svg
[Rust 1.66]: https://blog.rust-lang.org/2022/12/15/Rust-1.66.0.html
[borsh: rustc 1.67+]: https://img.shields.io/badge/rustc-1.67+-lightgray.svg
[Rust 1.67]: https://blog.rust-lang.org/2022/12/15/Rust-1.67.0.html
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: fix link if the msrv is bumped to 1.67

@frol
Copy link
Collaborator

frol commented Nov 8, 2023

@dj8yfo Thanks for staying on guard for users of borsh and avoiding raising requirements unnecessarily. This change as of now is totally fine with me.

@dj8yfo dj8yfo merged commit 2209e99 into near:master Nov 8, 2023
7 checks passed
@frol frol mentioned this pull request Nov 8, 2023
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