Skip to content

Add explanation of underscore-separated units in variable names [ci skip] #6470

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

Merged
merged 4 commits into from
Mar 18, 2025

Conversation

abyrd
Copy link
Member

@abyrd abyrd commented Feb 19, 2025

As discussed in a recent meeting, this adds a short explanation of why some variables have underscore-separated units as part of their names, defying standard camelCase.

It seems a little strange to update an already existing decision record, but this is a very minor point and probably less likely to be seen if it were split out into a separate style document.

@abyrd abyrd requested a review from a team as a code owner February 19, 2025 08:50
Copy link

codecov bot commented Feb 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.24%. Comparing base (c23f3a7) to head (6ebfa12).
Report is 417 commits behind head on dev-2.x.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #6470      +/-   ##
=============================================
+ Coverage      70.10%   70.24%   +0.13%     
- Complexity     18209    18368     +159     
=============================================
  Files           2073     2087      +14     
  Lines          77318    77381      +63     
  Branches        7820     7839      +19     
=============================================
+ Hits           54205    54356     +151     
+ Misses         20344    20250      -94     
- Partials        2769     2775       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@leonardehrenfried leonardehrenfried changed the title Add explanation of underscore-separated units in variable names Add explanation of underscore-separated units in variable names [ci skip] Feb 20, 2025
Copy link
Member

@optionsome optionsome left a comment

Choose a reason for hiding this comment

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

Perhaps a more fitting place would be NamingConventions.md.

@abyrd
Copy link
Member Author

abyrd commented Feb 24, 2025

Perhaps a more fitting place would be NamingConventions.md.

Yes, I considered that too. There was no section there on variables, but I could add one.

@abyrd abyrd requested a review from optionsome March 5, 2025 14:55

## Variables

We follow the Java standard of using camelCase with a lower-case first letter for variable names. However, variables of numeric types may include units as an underscore-separated suffix. For example, `length_mm` or `duration_msec`. The reason is that case is significant in metric prefixes (e.g. `M` for mega- versus `m` for milli-). The underscore also reinforces that these are units rather than part of the base variable name.
Copy link
Member

Choose a reason for hiding this comment

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

Idk if we should mention about the "edge case" of static fields where we also use underscore.

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you thinking of constants (static final int SOME_CONSTANT)? I'll reword it slightly to reference the Java Style Guide so all these cases are covered by reference.

@t2gran t2gran added this to the 2.8 (next release) milestone Mar 12, 2025
@abyrd abyrd requested review from t2gran and optionsome March 14, 2025 13:00
@t2gran t2gran merged commit b127aef into dev-2.x Mar 18, 2025
6 checks passed
@t2gran t2gran deleted the abyrd/units-identifiers branch March 18, 2025 12:50
@t2gran
Copy link
Member

t2gran commented Mar 18, 2025

Thank you for doing this @abyrd, I will merge it now.

t2gran pushed a commit that referenced this pull request Mar 18, 2025
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.

3 participants