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

split define_recv_packets! into a separate module #48

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

Conversation

CramBL
Copy link
Contributor

@CramBL CramBL commented Mar 9, 2025

Builds on top of #47

Part of addressing #19

split define_recv_packets! into a separate module as mentioned in a TODO.

The problem in the previous attempt (as described in the TODO) was related to the proc macros declaring the exact path to certain types like FieldIter and PacketSerializer.

You could "fix" it by adjusting the path to the new place where PacketSerializer now is, but the better choice is to avoid declaring such things in a macro and that's what I did.

Bump MSRV to 1.81.0

That's when "reasons" in allow lints was stabilized. I think it's an important feature for long term maintenance.

Added test.sh script

For convenience to check locally if CI is OK.

I would use just for these things if it was up to me. What do you think about using just?

@CramBL CramBL changed the title Split split define_recv_packets! into a separate module Mar 9, 2025
@andrei-ng
Copy link
Owner

@CramBL , I will merge this in main after I have a look how you solved the problem. But as per the discussion in the upstream repo, let's team up there.

@andrei-ng
Copy link
Owner

andrei-ng commented Mar 9, 2025

Bump MSRV to 1.81.0
That's when "reasons" in allow lints was stabilized. I think it's an important feature for long term maintenance.

Added test.sh script
For convenience to check locally if CI is OK.

I would use just for these things if it was up to me. What do you think about using just?

Thank you for the pointers!

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