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

Remove thiserror in favor of derive_more #50

Merged
merged 7 commits into from
Oct 23, 2023
Merged

Conversation

KillingSpark
Copy link
Owner

@KillingSpark KillingSpark commented Oct 14, 2023

This switches to derive_more so we get

  1. Nice formatting of the errors (both for no_std and std)
  2. Get to be no_std on stable instead of only nightly
  3. Still have the Error implementation if the std feature is enabled

Fixes: #47

@tamird
Copy link
Contributor

tamird commented Oct 18, 2023

LGTM. Is this ready to land?

@tamird
Copy link
Contributor

tamird commented Oct 18, 2023

You might want to change CI to test against stable rather than nightly:

- name: Install nightly toolchain
uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: nightly
override: true
- name: Run cargo check without std feature
uses: actions-rs/cargo@v1
with:
command: check
toolchain: nightly
args: --no-default-features
test-no-std:
name: Test Suite (no_std)
runs-on: ubuntu-latest
steps:
- name: Checkout sources
uses: actions/checkout@v2
- name: Install nightly toolchain
uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: nightly
override: true
- name: Run cargo test without std feature
uses: actions-rs/cargo@v1
with:
command: test
toolchain: nightly
args: --no-default-features
lints-no-std:
name: Lints (no std)
runs-on: ubuntu-latest
steps:
- name: Checkout sources
uses: actions/checkout@v2
- name: Install nightly toolchain
uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: nightly
override: true
components: rustfmt, clippy
- name: Run cargo fmt
uses: actions-rs/cargo@v1
with:
command: fmt
toolchain: nightly
args: --all -- --check
- name: Run cargo clippy
uses: actions-rs/cargo@v1
with:
command: clippy
toolchain: nightly
args: --no-default-features -- -D warnings

@tamird
Copy link
Contributor

tamird commented Oct 18, 2023

Dang, derive_more also depends on syn@1. Severing that dependency edge was my motivation for #48.

They bumped the dependency in JelteF/derive_more@1fb07f1 but it is not yet released.

@KillingSpark
Copy link
Owner Author

Dang, derive_more also depends on syn@1

Yep but they are having beta releases for a 1.0 which makes ma hopeful that this will soon be rectified

I'm pretty busy right now, I'll try and merge this next week

@KillingSpark
Copy link
Owner Author

Okay so I think the way I'm going to handle this is merging this, and bumping derive_more to 1.0 once they release it.

This will allow all three goals above to be reached now and gives a clear-ish roadmap (wait for derive_more 1.0 release) towards not compiling syn 1.0 AND syn 2.0 in the future.

@KillingSpark KillingSpark merged commit e620d2a into master Oct 23, 2023
12 checks passed
@tamird
Copy link
Contributor

tamird commented Oct 23, 2023

Cool, thanks. Will you cut a new release with this change?

@KillingSpark
Copy link
Owner Author

Today @tamird ;)

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.

Crate isn't really no-std compatible
2 participants