Improve code base quality - #117
Merged
Merged
Conversation
We still had some __thread storage class specifier in the code base. Change it to the standard _Thread_local. Signed-off-by: Alessandro Pellegrini <a.pellegrini@ing.uniroma2.it>
They are not improving any performance though. Signed-off-by: Alessandro Pellegrini <a.pellegrini@ing.uniroma2.it>
Align __typeof to __typeof__ Signed-off-by: Alessandro Pellegrini <a.pellegrini@ing.uniroma2.it>
Improve readability by using more readable variable names. Signed-off-by: Alessandro Pellegrini <a.pellegrini@ing.uniroma2.it>
Documenting several functions/datatypes with missing documentation, and fixing some documentation errors. Signed-off-by: Alessandro Pellegrini <a.pellegrini@ing.uniroma2.it>
There was an old email address in the code of conduct. Signed-off-by: Alessandro Pellegrini <a.pellegrini@ing.uniroma2.it>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #117 +/- ##
===========================================
+ Coverage 84.35% 86.36% +2.00%
===========================================
Files 43 45 +2
Lines 1726 1569 -157
Branches 54 41 -13
===========================================
- Hits 1456 1355 -101
+ Misses 250 197 -53
+ Partials 20 17 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Documentation coverage is 92.5% 👍 |
|
Documentation coverage is 92.5% 👍 |
Documenting several functions/datatypes with missing documentation, and fixing some documentation errors. Signed-off-by: Alessandro Pellegrini <a.pellegrini@ing.uniroma2.it>
|
Documentation coverage is 94.6% 👍 |
This commit moves around functions and data structures, based on the following core ideas: - Buddy system stuff are confined in mm/buddy/ files - model_allocator.c should implement everything that can be seen by the model or pertains to the LP state memory. It interacts with the buddy system, so the concept of "multi.c" is not necessary, nor pertains to mm/buddy/. Out of mm/ no one should explicitly see the buddy system. - in mm/checkpoint we have everything that relates to checkpointing of the LP state. So we now have an `autonomic.c` unit, a `full.c` unit, and an `incremental.c` unit, that is currently only keeping old code. `mm/checkpoint/checkpoint.h` is the only header exposing all checkpointing-related stuff, so independently of what you need (autonomic/full/incremental), you can include this header. I am convinced this refactor helps people go and find stuff based on the organization of the tree. There could be a minimal penaly, as we have lost one `static inline` for a function, but I am happy tolerating a minimal performance penalty in favor of readability. Signed-off-by: Alessandro Pellegrini <a.pellegrini@ing.uniroma2.it>
alessandropellegrini
force-pushed
the
code-cleanup
branch
from
May 27, 2025 15:52
a1c73be to
06915e2
Compare
|
Documentation coverage is 94.9% 👍 |
Ensure that all files are adhering to the ROOT-Sim code formatting style. There are some exceptions that have been discussed in the past, that I am keeping as is as they are more readable. Signed-off-by: Alessandro Pellegrini <a.pellegrini@ing.uniroma2.it>
|
Documentation coverage is 94.6% 👍 |
In the mm refactor, a renamed function was not renamed in the corresponding header. Signed-off-by: Alessandro Pellegrini <a.pellegrini@ing.uniroma2.it>
|
Documentation coverage is 94.9% 👍 |
This is dumb, because _we_ manage the actions, but yes, our code is waaaay more secure now. Signed-off-by: Alessandro Pellegrini <a.pellegrini@ing.uniroma2.it>
|
Documentation coverage is 94.9% 👍 |
alessandropellegrini
marked this pull request as ready for review
May 29, 2025 17:24
Piccions
reviewed
May 30, 2025
Piccions
reviewed
May 30, 2025
Piccions
reviewed
May 30, 2025
Piccions
reviewed
Jun 2, 2025
Piccions
requested changes
Jun 2, 2025
In stdlib there is the deprecated index(3) function which has been deprecated in favour of strchr(3). Avoid using index as the name of the variable to avoid any possible issues. Signed-off-by: Alessandro Pellegrini <a.pellegrini@ing.uniroma2.it>
Fix wrong documentation. Signed-off-by: Alessandro Pellegrini <a.pellegrini@ing.uniroma2.it>
I confused by mistake the message processing data with an LP context. This is fixed, and the arguments names are made consistent in the code base. Signed-off-by: Alessandro Pellegrini <a.pellegrini@ing.uniroma2.it>
This is made to align to the project conventions. Signed-off-by: Alessandro Pellegrini <a.pellegrini@ing.uniroma2.it>
|
Documentation coverage is 94.9% 👍 |
Member
Author
|
I have fixed all your suggestions, thanks! Regarding the |
Piccions
approved these changes
Jun 25, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR attempts to fix minor nits in the code base. More importantly, I am trying to improve the overall documentation coverage.