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

Syn/2.0 #3563

Merged
merged 4 commits into from
Mar 24, 2023
Merged

Syn/2.0 #3563

merged 4 commits into from
Mar 24, 2023

Conversation

weiznich
Copy link
Member

This updates our dependency on syn to the latest 2.0 release. Also it removes the proc_macro_error crate as that requires syn 1.x

@weiznich weiznich force-pushed the syn/2.0 branch 2 times, most recently from e7e51e0 to d725412 Compare March 24, 2023 13:01
as that will fail due to the use of the nightly feature
@pksunkara
Copy link
Member

Would you mind if we use a forked version of proc_macro_error? We need that for clap also.

@@ -445,7 +445,7 @@ jobs:
sudo apt-get update
sudo apt-get -y install libsqlite3-dev libpq-dev libmysqlclient-dev
- name: Check diesel_derives
run: cargo +1.65.0 minimal-versions check -p diesel_derives --all-features
run: cargo +1.65.0 minimal-versions check -p diesel_derives --features "postgres sqlite mysql 32-column-tables 64-column-tables 128-column-tables without-deprecated with-deprecated r2d2"
Copy link
Member

Choose a reason for hiding this comment

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

Why not? Wouldn't we want to check that all features work on minimal dep versions?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because --all-features would enable the nightly feature for diesel-derives, which in turn would enable an unstable rustc feature, which in turn will not compile on a stable release.
The feature list is equivalent to --all-features with the nightly feature removed.

@weiznich
Copy link
Member Author

Would you mind if we use a forked version of proc_macro_error

I'm fine with using a forked version of that crate as long as:

  • It's maintained by someone I trust. (Which is the case for the clap team for example)
  • It's ready before we cut the next release.

There is another reasons why I moved away from proc_macro_error: The old implementation used to generate panic's to generate error messages in quite a few cases. That's not considered good practice anymore. You should rather use compile_error! instead. That's fixed by this PR, by having results and moving the error handling up to the top level.

@pksunkara
Copy link
Member

I am willing to maintain it in either this org or clap org.

But before I start doing that, I would like to ask @epage's opinion first because clap is going to have the same issue.

@epage
Copy link

epage commented Mar 24, 2023

Would you mind if we use a forked version of proc_macro_error? We need that for clap also.

I already removed proc-macro-error from clap in clap-rs/clap#4777

@pksunkara pksunkara added this pull request to the merge queue Mar 24, 2023
Merged via the queue into diesel-rs:master with commit e548447 Mar 24, 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