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

removed outdated dependency derivative #351

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

Conversation

xNxExOx
Copy link

@xNxExOx xNxExOx commented Feb 22, 2024

No description provided.

@xNxExOx xNxExOx requested review from estk and gadunga as code owners February 22, 2024 06:45
@bconn98
Copy link
Collaborator

bconn98 commented Feb 24, 2024

I'll take a look at this either today or tomorrow. Thanks for the PR!

Copy link
Collaborator

@bconn98 bconn98 left a comment

Choose a reason for hiding this comment

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

Otherwise looks great! And please run the formatter locally

CHANGELOG.md Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Feb 28, 2024

Codecov Report

Attention: Patch coverage is 13.04348% with 20 lines in your changes are missing coverage. Please review.

Project coverage is 62.69%. Comparing base (f688e38) to head (f8b06d6).

Files Patch % Lines
src/append/console.rs 0.00% 5 Missing ⚠️
src/append/rolling_file/mod.rs 0.00% 5 Missing ⚠️
src/encode/mod.rs 0.00% 4 Missing ⚠️
src/append/file.rs 0.00% 3 Missing ⚠️
src/encode/pattern/mod.rs 0.00% 3 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #351      +/-   ##
==========================================
- Coverage   63.42%   62.69%   -0.73%     
==========================================
  Files          25       25              
  Lines        1572     1595      +23     
==========================================
+ Hits          997     1000       +3     
- Misses        575      595      +20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bconn98
Copy link
Collaborator

bconn98 commented Feb 28, 2024

@xNxExOx I've got a lot of the CI fixes running in my test branch. Clippy went and changed some rules on us and then toml upped their MSRV to 1.70. If you're okay waiting for that branch, you can wait it out. Otherwise see if you can tackle the Clippy warnings and update the MSRV (Cargo.toml, README, and main.yml in the workflows).

@bconn98
Copy link
Collaborator

bconn98 commented Feb 28, 2024

Test coverage updates are always appreciated. But I've got 2000+ lines of test code that should cover a majority of the project that is missing it.

@bconn98
Copy link
Collaborator

bconn98 commented Mar 1, 2024

@xNxExOx #354 is out with just the changes to fix the builds if your interested. Talking with the owner to get that merged in ASAP

@bconn98
Copy link
Collaborator

bconn98 commented Mar 3, 2024

Fixes are in main now. @xNxExOx

@estk
Copy link
Owner

estk commented Apr 1, 2024

This lgtm, @xNxExOx can you get CI passing and we'll get this merged?

@xNxExOx
Copy link
Author

xNxExOx commented Apr 1, 2024

😕 does these checks pass on the main branch? because they seems totally unrelared to anything I changed.

@bconn98
Copy link
Collaborator

bconn98 commented Apr 2, 2024

Agreed, this is stalled on the deprecated API

@bconn98
Copy link
Collaborator

bconn98 commented Jul 6, 2024

No need to swap to this, but was checking in on alternatives and it looks like the rust language proper just swapped from derivative to https://crates.io/crates/derive-where.

@xNxExOx
Copy link
Author

xNxExOx commented Jul 7, 2024

I see only that the job failed, but no idea why, so I am not looking into it.

@bconn98
Copy link
Collaborator

bconn98 commented Jul 7, 2024

Yup no worries. Working on getting the fix into main. Should happen this weekend or next.

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.

4 participants