Repository navigation
Improvement: Use a fixed constant for the length of the description field in t8_vtk_data_field_t - #2499
Improvement: Use a fixed constant for the length of the description field in t8_vtk_data_field_t#2499benegee wants to merge 4 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2499 +/- ##
=======================================
Coverage 82.79% 82.79%
=======================================
Files 139 139
Lines 21581 21581
=======================================
Hits 17869 17869
Misses 3712 3712 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
holke
left a comment
There was a problem hiding this comment.
Looks good, thanks :)
Just a minor remark to add an explanation for the 1023.
|
|
||
| #define T8_VTK_FORMAT_STRING "ascii" /**< Format string for vtk */ | ||
| #define T8_VTK_FORMAT_STRING "ascii" /**< Format string for vtk */ | ||
| #define T8_VTK_MAX_STRING_LENGTH 1023 /**< Maximal string length for the description in t8_vtk_data_field_t */ |
There was a problem hiding this comment.
Could you please add a comment in the code why we choose 1023 - since its a magic number, we should have an explanation for it.
There was a problem hiding this comment.
Embarrassing...
It had been 1024. 1K, just because I like the number, and I thought 8K (as seen on Linux) is a bit too much.
I then did a quick check to make sure I could actually retrieve the number from an external application, changed the number, and forgot to revert the change.
There was a problem hiding this comment.
While we are at it, naming is hard. Do you have other suggestions for the macro name?
|
Also, please adhere to the PR naming conventions: " The title starts with one of the following prefixes: Documentation:, Bugfix:, Feature:, Improvement: or Other:." |
Describe your changes here:
Resolves #2454
All these boxes must be checked by the AUTHOR before requesting review:
Documentation:,Bugfix:,Feature:,Improvement:orOther:.All these boxes must be checked by the REVIEWERS before merging the pull request:
As a reviewer please read through all the code lines and make sure that the code is fully understood, bug free, well-documented and well-structured.
General
Tests
If the Pull request introduces code that is not covered by the github action (for example coupling with a new library):
Scripts and Wiki
scripts/internal/find_all_source_files.shto check the indentation of these files.License
doc/(or already has one).