Added time measurements #35
Conversation
src/UnitConverter/Unit/Time/Hour.php
Outdated
|
|
||
| declare(strict_types = 1); | ||
|
|
||
| namespace UnitConverter\Unit\Volume; |
There was a problem hiding this comment.
Fix namespace (Volume should be Time).
src/UnitConverter/Unit/Time/Hour.php
Outdated
|
|
||
| use UnitConverter\Unit\{ | ||
| Time\TimeUnit | ||
| }; |
There was a problem hiding this comment.
Remove superfluous import statement (see notes at the end on TimeUnit.php as to why this is not needed).
|
|
||
| use UnitConverter\Unit\{ | ||
| Time\TimeUnit | ||
| }; |
There was a problem hiding this comment.
Same as Hour.php – Fix namespace, and remove unnecessary import statement for Time\TimeUnit.
|
|
||
| ->setSymbol("ms") | ||
|
|
||
| ->setUnits(.001) |
There was a problem hiding this comment.
Tack a 0 onto the left off that decimal place for the sake of readability. :)
|
|
||
| use UnitConverter\Unit\{ | ||
| Time\TimeUnit | ||
| }; |
There was a problem hiding this comment.
Same as Hour.php – Fix namespace, and remove unnecessary import statement for Time\TimeUnit.
|
|
||
| use UnitConverter\Unit\{ | ||
| Time\TimeUnit | ||
| }; |
There was a problem hiding this comment.
Same as Hour.php – Fix namespace, and remove unnecessary import statement for Time\TimeUnit.
| }; | ||
|
|
||
| /** | ||
| * Gallon unit data class. |
There was a problem hiding this comment.
Fix docblock to reference correct unit
| use UnitConverter\Measure; | ||
| use UnitConverter\Unit\{ | ||
| AbstractUnit, Volume\Second | ||
| }; |
There was a problem hiding this comment.
This can reduced to simply,
use UnitConverter\Measure;
use UnitConverter\Unit\AbstractUnit;Since Second.php lives in the same namespace as TimeUnit, we don't need to specify a namespace path to import the class – it can simply be used as is, by all classes within the same namespace (this goes for all unit classes above that are importing Time\TimeUnit as well) 😃
|
|
||
| ->setSymbol("month") | ||
|
|
||
| ->setUnits(2678400) |
There was a problem hiding this comment.
Where did you get this number? This isn't what I found from Googles' converter and other random converters. They all land between 2,628,000 and 2,629,000.
1 month === 2.682e+6 seconds
2.682e+6 = (2.628 * pow(10,6))
= 2,628,000 seconds
jordanbrauer
dismissed
their stale review
Not really necessary to be changed, however worth a converstaion, as time units are going to be wonky similarly to temperature
* remove composer.lock from repo and add it to .gitignore (#30) * Implement Kilopascal and Megapascal and write tests (#29) * implement Kilopascal * implement Megapascal * fix docblocks * Fix: add missing self conversions for temperature units (#31) * Feature/improve travis file (#32) * update .travis.yml * Added time measurements (#35) Closes #10 * Added time measurements * Added requested changes * Added new time units * Feat: add simple contributing guide for now (#36) * Added last weight measures (#37) * Added last weight measures * Fixed stone units * Added millibar as pressure unit (#34) * Added millibar as pressure unit * Fixed wrong millibar, also fixed documentation block * Added requested changes * Added requested changes * Fixed millibar units * scientificSymbol property, getter and setter are added (#47) closes #46 * Add first set of dev docs (#44) * Update: composer docs cmd now uses bash script to check for stale docs * Update: phpDocumentor XML config file * Feat: docs for v0.3.9-alpha (#43) * Update: move docs directory to project root for github pages support * Adding energy units (#33) * Properly casing unit symbols * Adding requested energy units * Removing unnecessary imports * Upgrade: docs & composer version * Fix: Energy integration test - wrong unit case for Joule
Added the basic time measurement functions.
I'd be willing to add more if these ones are good. Let me know