ProfessionalWiki / EDTF

PHP library to parse, represent and work with dates that follow the Extended Date/Time Format specification.
https://wikibase.consulting/wikibase-edtf
GNU General Public License v2.0
10 stars 7 forks source link

Fix Seasons in Intervals #64

Closed chaudbak closed 3 years ago

chaudbak commented 3 years ago

Introduced a couple of new interfaces:

  1. SimpleEdtf - some simple edtf (extdate, season) which can exist on it's own or as a part of complex structures (sets, intervals).
  2. Coverable - something, that can be checked with covers() method
codecov-commenter commented 3 years ago

Codecov Report

Merging #64 (342fc71) into master (1267dbb) will decrease coverage by 0.49%. The diff coverage is 72.72%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #64      +/-   ##
============================================
- Coverage     94.29%   93.80%   -0.50%     
- Complexity      506      510       +4     
============================================
  Files            31       31              
  Lines          1104     1113       +9     
============================================
+ Hits           1041     1044       +3     
- Misses           63       69       +6     
Flag Coverage Δ Complexity Δ
phpunit 93.80% <72.72%> (-0.50%) 510.00 <32.00> (+4.00) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ Complexity Δ
src/Model/ExtDate.php 97.47% <0.00%> (-1.67%) 59.00 <1.00> (+1.00) :arrow_down:
src/Model/ExtDateTime.php 86.04% <ø> (ø) 18.00 <0.00> (ø)
src/Model/Season.php 75.83% <0.00%> (-2.62%) 74.00 <2.00> (+2.00) :arrow_down:
src/Model/Interval.php 89.18% <100.00%> (ø) 21.00 <2.00> (ø)
src/Model/IntervalSide.php 100.00% <100.00%> (ø) 8.00 <2.00> (ø)
src/Model/Set.php 93.10% <100.00%> (ø) 16.00 <6.00> (ø)
src/PackagePrivate/CoversTrait.php 100.00% <100.00%> (ø) 2.00 <2.00> (ø)
...gePrivate/Humanizer/InternationalizedHumanizer.php 97.93% <100.00%> (ø) 47.00 <5.00> (ø)
...gePrivate/Humanizer/PrivateStructuredHumanizer.php 97.91% <100.00%> (ø) 21.00 <3.00> (ø)
src/PackagePrivate/Parser/Parser.php 98.70% <100.00%> (+0.01%) 74.00 <7.00> (+1.00)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1267dbb...342fc71. Read the comment docs.

JeroenDeDauw commented 3 years ago

You can turn EdtfValue into an interface. Then SimpleEdtf can extend that interface. Then the static analysis won't get mad when something that implements SimpleEdtf gets passed to a function expecting EdtfValue.

chaudbak commented 3 years ago

You can turn EdtfValue into an interface. Then SimpleEdtf can extend that interface. Then the static analysis won't get mad when something that implements SimpleEdtf gets passed to a function expecting EdtfValue.

To be honest, I don't see the reason in specified type for humanize() argument. Because humanize() method implementation anyway checks the type of passed argument (via instanceof). Looks like we need it here just to "shut up" the Psalm check :). As a quick fix I propose to keep the $edtf argument untyped, place the psalm annotation to ignore this thing and get back to this later later as a technical improvement. I suspect this humanize() method anyway has a bad-design smell and it must be refactored which is not the priority at the moment (furthermore, it may take a lot of time to refactor it). What do you think?