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

change derivative to reduce to fix clippy issues #437

Closed
wants to merge 1 commit into from

Conversation

YJDoc2
Copy link
Contributor

@YJDoc2 YJDoc2 commented Oct 9, 2024

ref #303 (comment)

This changes derivative to educe with the intention of fixing clippy failures.

Unfortunately I had to manually implement a Debug at one place to maintain the formatting as educe does not have equivalent to derivative's transparent option. It seems like we have switched one unmaintained crate to another almost unmaintained crate 🙃 . Atleast educe is more up-to-date and should fix the clippy. I checked few other options, but educe is by far the most compatible with derivative in terms of options.

Also changed CONTRIBUTING to mention opening an issue instead of mailing imsnif , as that seems more appropriate now.

@YJDoc2
Copy link
Contributor Author

YJDoc2 commented Oct 9, 2024

Thanks, I'll push with the Changelog updated when running CIs are finished.

@cyqsimon
Copy link
Collaborator

cyqsimon commented Oct 9, 2024

It seems like we have switched one unmaintained crate to another almost unmaintained crate 🙃 .

Yeah. Looking at the repo, the author seems MIA. Maybe derive_more is the way to go after all. Its Debug macro offers transparency and custom formatter, not to mention the way it does it is nicer too, not needing a standalone function. The lack of a customisable Default macro, well I should just quit bitching about it TBH. What matters is it actually has proper maintenance and governance.

So what do you think?

@cyqsimon
Copy link
Collaborator

cyqsimon commented Oct 9, 2024

And I'll update the contact info stuff separately from this PR, just to keep things clean.

@YJDoc2
Copy link
Contributor Author

YJDoc2 commented Oct 14, 2024

Maybe derive_more is the way to go after all. Its Debug macro offers transparency and custom formatter, not to mention the way it does it is nicer too, not needing a standalone function.

Sure, I'll probably close this PR and open another one for changing to derive_more.

So what do you think?

I agree with the part that having a properly maintained crate is better, I am wondering if we are implementing the Default macro by ourselves, should we go all the way and implement the Debug as well? We would remove the dependency completely.I;m not sure if that would be much more complex/harder to maintain these manual implementations. WDYT? I'll proceed accordingly.

And I'll update the contact info stuff separately from this PR, just to keep things clean.

Yep, thanks!

@cyqsimon
Copy link
Collaborator

I agree with the part that having a properly maintained crate is better, I am wondering if we are implementing the Default macro by ourselves, should we go all the way and implement the Debug as well? We would remove the dependency completely.I;m not sure if that would be much more complex/harder to maintain these manual implementations. WDYT? I'll proceed accordingly.

Let's go with derive_more. Many of its other macros are very useful as well, so I suspect I'll run into a reason to pull it in in the future regardless.

@YJDoc2
Copy link
Contributor Author

YJDoc2 commented Oct 15, 2024

Let's go with derive_more. Many of its other macros are very useful as well, so I suspect I'll run into a reason to pull it in in the future regardless.

ok, makes sense 👍 I'll close this PR and open another one for that soon.

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.

2 participants