Added a new volume unity Oil Barrels and a new permeability class that handles Darcy and MiliDarcys.
BUG: 388074
FIXED-IN: 5.53
Details
Details
- Reviewers
ngraham broulik - Group Reviewers
Frameworks - Commits
- R292:b2419e57eeea: Updated with petroleum industry units
Diff Detail
Diff Detail
- Repository
- R292 KUnitConversion
- Branch
- permeability (branched from master)
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 4590 Build 4608: arc lint + arc unit
Comment Actions
Great, thanks! This work well; I just have a few comments, listed below.
To update this diff without accidentally creating a new one, make sure you're on the permeability branch in your kunitconversion repo, then make your change and run arc diff. It shouldn't try to create a new one.
| src/unit.h | ||
|---|---|---|
| 333 ↗ | (On Diff #44903) | Can you add /** @since 5.53 */ above each new one, following the pattern? |
| src/volume.cpp | ||
| 452 ↗ | (On Diff #44903) | I'm not sure this should be marked as a common unit. Because it is, I now get oil barrels listed first when I have KRunner convert something like "2 cups" or "8 liters". |
| src/unit.h | ||
|---|---|---|
| 65 ↗ | (On Diff #44903) | Please add /** @since 5.53 */ here, following the pattern. |
Comment Actions
The header file for class Permeability should probably be named permeability_p.h to match the other category classes since it's a not exported implementation detail
| src/unit.h | ||
|---|---|---|
| 332 ↗ | (On Diff #44903) | Given this is a new category, assign Darcy = 33000 so that one can still add new values after Yoctoohm to electrical resistance category without breaking compatibility |
Comment Actions
Looks great and works great! Nice first patch. Just one more thing is required before I can land it: add the following to the Summary section of the patch:
BUG: 388074 FIXED-IN: 5.53
Thanks!