maybe-async-rs icon indicating copy to clipboard operation
maybe-async-rs copied to clipboard

Add `legacy-syntax` and `syn-2` features

Open smoelius opened this issue 2 years ago • 6 comments

This PR does essentially two things:

  • Adds an enabled-by-default feature legacy-syntax and gates the test macro behind that feature
  • Adds a syn-2 feature to enabled use of syn version 2

gitoxide brings in two copies of syn, version 1 and version 2. Relying on maybe-async is the reason it needs version 1.

maybe-async cannot be easily converted to use syn version 2. maybe-async's test macro uses async as a nested attribute name. However, Rust 2018 added async as a keyword, and keywords cannot be used as attribute names. For this reason, syn does not parse them. One could try to work around the syn limitation, but one could also make a case that async should be changed to something else.

Despite all this, gitoxide does not use the test macro.

So, with the features described above, gitodixde can use maybe-async with default-features = false, features = ["syn-2"] and not have to bring in syn version 1.


I have not discussed this with the gitoxide maintainers. If you prefer, I could first get confirmation from them that they like this plan.

Also, I realize this is out of the blue. Feel free to nit anything, or close this PR if you don't like the idea.

smoelius avatar Nov 03 '23 23:11 smoelius

I think it fine to just dump the maybe_async::test attribute macro, since it is just a alias for a set of cfg_attr. We just need to clearly state how to use cfg_attr to add test in docs.

But unfortunately this introduces a breaking change in API, so I will release a minor version bump (v0.3).

fMeow avatar Dec 12 '23 07:12 fMeow

We just need to clearly state how to use cfg_attr to add test in docs.

You would state that in CHANGELOG.md, or somewhere else?

smoelius avatar Dec 12 '23 10:12 smoelius

Both README.md, CHANGELOG.md and docs in rust code so that we can see it in docs.rs.

fMeow avatar Dec 13 '23 08:12 fMeow

Just to be clear, the whole package gets upgraded to syn version 2, correct?

I..e, there should be no syn-1 feature to allow continued use of syn version 1, correct?

smoelius avatar Dec 13 '23 11:12 smoelius

Hi @smoelius, thanks for working on this! (I'm just a random user.) I'm looking at importing this crate into Android and while we do have a copy of syn version 1, we would like to remove it in the future. So it would be great if maybe-async would no longer depend on the old version.

mgeisler avatar Dec 28 '23 15:12 mgeisler

Thanks for your feedback, @mgeisler.

If this PR is merged, I expect it to be possible to use the crate without relying on syn version 1. I expect that independent of how @fMeow responds regarding a syn-1 feature,

smoelius avatar Dec 28 '23 17:12 smoelius

The PR #23 fully tackle the test macro problem. I will close this PR.

fMeow avatar Jan 30 '24 10:01 fMeow