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

📝 Add documentation to packet_falcon_core #24

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

GrizzlT
Copy link
Member

@GrizzlT GrizzlT commented Oct 8, 2022

Apart from the derive macros, all items should be documented. Since this is the crate that does not depend on any other falcon crate, this should be understandable on its own.

Is that the case, is this clear and sufficient documentation?

@GrizzlT GrizzlT marked this pull request as ready for review October 10, 2022 17:05
Copy link
Collaborator

@VixieTSQ VixieTSQ left a comment

Choose a reason for hiding this comment

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

These are all nitpicks and absolutely not blockers. But what we discussed before is still a problem. I could not get a solid grasp on what the "seed-flavored" traits are really meant for and the external input stuff needs to be explained better.

Comment on lines +5 to +6
/// Helper type to write iterators over implementors of the seed-less flavored
/// traits to a minecraft connection.
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does it mean to write iterators to a Minecraft connection? As a Bytes? Each individually?

Copy link
Member Author

Choose a reason for hiding this comment

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

Iterators over PacketWrite, meaning iterators over more complex types. U should use the specialized implementations for byte arrays.

Comment on lines +19 to +21
#[doc = "Transparent wrapper around an "]
#[doc = stringify!($base)]
#[doc = "."]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#[doc = "Transparent wrapper around an "]
#[doc = stringify!($base)]
#[doc = "."]
#[doc = concat!("Transparent wrapper around an ", stringify!($base), "."]

This might be more readable

@@ -36,13 +42,10 @@ macro_rules! impl_var_int {
}

impl $var {
/// Return the stored value.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// Return the stored value.
/// Return the inner value.

Sounds more rusty

crates/packet_core_derive/src/lib.rs Show resolved Hide resolved
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