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

Rust doc: add docstring to rust module files - v1 #9579

Closed
wants to merge 1 commit into from

Conversation

0xEniola
Copy link
Contributor

@0xEniola 0xEniola commented Oct 8, 2023

Link to redmine ticket: https://redmine.openinfosecfoundation.org/issues/4584

Describe changes:

  • Added docstrings for the short descriptions of Rust module files.

@jufajardini jufajardini added the outreachy Contributions made by Outreachy applicants label Oct 8, 2023
@github-actions
Copy link

github-actions bot commented Oct 8, 2023

NOTE: This PR may contain new authors:

Daniel Olatunji <danielolatunji20@outlook.com>

Copy link
Contributor

@jufajardini jufajardini left a comment

Choose a reason for hiding this comment

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

nit: our commit messages aren't usually started with capital letter, can you please use something like rust/doc?

For the mod file, could you please add something that indicates that? Eg. Template Application layer module, with the necessary rewordings, of course :P

There are lots of CI failures, but the ones I checked seemed connected to an inline comment I've left, so I'd suggest addressing that and seeing if things improve. :)

Comment on lines +31 to +32
//! Gap handling and Chunk-based file transfer tracker.

Copy link
Contributor

Choose a reason for hiding this comment

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

cargo clippy isn't happy with this doc comment (cf error[E0753]: expected outer doc comment). I'm not sure why, but I'm wondering if that's because we have a previous comment before.

I would suggest either removing this, as this module has a description, already, or moving this doc comment to before the file and author comment...

@0xEniola
Copy link
Contributor Author

0xEniola commented Oct 8, 2023

nit: our commit messages aren't usually started with capital letter, can you please use something like rust/doc?

For the mod file, could you please add something that indicates that? Eg. Template Application layer module, with the necessary rewordings, of course :P

There are lots of CI failures, but the ones I checked seemed connected to an inline comment I've left, so I'd suggest addressing that and seeing if things improve. :)

OK, you mean for each mod file, to indicate that it is a mod file, yeah?

@0xEniola
Copy link
Contributor Author

0xEniola commented Oct 8, 2023

nit: our commit messages aren't usually started with capital letter, can you please use something like rust/doc?

For the mod file, could you please add something that indicates that? Eg. Template Application layer module, with the necessary rewordings, of course :P

There are lots of CI failures, but the ones I checked seemed connected to an inline comment I've left, so I'd suggest addressing that and seeing if things improve. :)

You mean something like; FTP Application layer parser and logger module?

Like this?

@jufajardini
Copy link
Contributor

nit: our commit messages aren't usually started with capital letter, can you please use something like rust/doc?
For the mod file, could you please add something that indicates that? Eg. Template Application layer module, with the necessary rewordings, of course :P
There are lots of CI failures, but the ones I checked seemed connected to an inline comment I've left, so I'd suggest addressing that and seeing if things improve. :)

OK, you mean for each mod file, to indicate that it is a mod file, yeah?

Yes, basically :)

@jufajardini
Copy link
Contributor

nit: our commit messages aren't usually started with capital letter, can you please use something like rust/doc?
For the mod file, could you please add something that indicates that? Eg. Template Application layer module, with the necessary rewordings, of course :P
There are lots of CI failures, but the ones I checked seemed connected to an inline comment I've left, so I'd suggest addressing that and seeing if things improve. :)

You mean something like; FTP Application layer parser and logger module?

Like this?

Yup, looks better to me :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
outreachy Contributions made by Outreachy applicants
2 participants