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

adding some basic documentation for auraed source code #504

Merged
merged 1 commit into from
May 29, 2024

Conversation

taniwha3
Copy link
Contributor

Signed-off-by: Tani Aura <111664369+taniwha3@users.noreply.github.com>
macro_rules! do_in_cell {
($self:ident, $cell_name:ident, $function:ident, $request:ident) => {{
// Lock the cells mutex to ensure exclusive access
Copy link
Contributor

Choose a reason for hiding this comment

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

in general I'm not a huge fan of comments that describe the code that is already readable. I prefer to keep comments for methods/classes and any tricky bits of code. that way readers know to pay attention to the comments that exist and they don't become background noise that readers ignore. does that make sense?

@@ -145,6 +170,13 @@ impl CellService {
})
}

/// Frees a cell. Will delocate the cell.
Copy link
Contributor

Choose a reason for hiding this comment

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

"deallocate"?

@GANitak
Copy link
Contributor

GANitak commented May 28, 2024

FWIW I think the commentary lines can only help understand the step better for someone cruising the code without having a large amount of experience with this kind of code, or language.
So it helps me, yes some of the lines are obvious, but others are not.
Concerning the background noise, I can see that too.
Maybe there is a way to hide comment sections somehow...?
But more accessible information directly from the code base, a gain of time for the people.
Can comments affect runtime seriously?

@dmah42 dmah42 merged commit 66246e7 into aurae-runtime:main May 29, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants