ProfessionalWiki / AutomatedValues

Wikibase extension that allows defining rules to automatically set labels or aliases based on Statement values
https://wikibase.consulting/automating-values-in-wikibase
GNU General Public License v2.0
6 stars 3 forks source link

Compatibility fix for WB1.38 #26

Closed QuentinJanuel closed 1 year ago

QuentinJanuel commented 1 year ago

I implemented the compatibility fix for WB1.38 as described in #20

JeroenDeDauw commented 1 year ago

Static analysis error:

Error: Method ProfessionalWiki\AutomatedValues\Compat::newPId() should return Wikibase\DataModel\Entity\PropertyId but returns Wikibase\DataModel\Entity\NumericPropertyId.

JeroenDeDauw commented 1 year ago

Can likely be fixed by running Psalm and PHPStan with MW 38, so they know that NumericProperyId implements ProperyId.

You can modify that in https://github.com/ProfessionalWiki/AutomatedValues/blob/master/.github/workflows/ci.yml. There you can also make the unit tests run with MW 38.

QuentinJanuel commented 1 year ago

Thanks for the suggestion, sadly it still doesn't work because AutomatedValues still doesn't seem fully compatible with WB38. For instance here is the error for PHPStan: error

codecov-commenter commented 1 year ago

Codecov Report

Merging #26 (faf763d) into master (968c94d) will decrease coverage by 2.62%. The diff coverage is 33.33%.

:exclamation: Current head faf763d differs from pull request most recent head bc148e5. Consider uploading reports for the commit bc148e5 to get more accurate results

@@             Coverage Diff              @@
##             master      #26      +/-   ##
============================================
- Coverage     87.69%   85.07%   -2.62%     
- Complexity      126      131       +5     
============================================
  Files            18       19       +1     
  Lines           260      268       +8     
============================================
  Hits            228      228              
- Misses           32       40       +8     
Impacted Files Coverage Δ
src/Compat.php 0.00% <0.00%> (ø)
src/DataAccess/RulesDeserializer.php 100.00% <100.00%> (ø)
src/Domain/StatementEqualityCriterion.php 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

QuentinJanuel commented 1 year ago

Hey, I think I solved the PHPStan and Psalm issues, but meanwhile I cleared the github workflow caches and now PHPUnit: MW REL1_35, PHP 7.4 fails at Run composer update with

Error: Failed to clone https://github.com/wikimedia/avro-php.git via https, ssh protocols, aborting.

Indeed, this repo seems to no longer exist. Do you have any suggestion of what I should do?

JeroenDeDauw commented 1 year ago

Don't forget to update https://github.com/ProfessionalWiki/AutomatedValues/blob/master/.github/workflows/ci.yml#L14-L19. Could perhaps include MW 39 right away

JeroenDeDauw commented 1 year ago

You can include the Compat.php file from analysis via https://github.com/ProfessionalWiki/AutomatedValues/blob/master/phpstan.neon#L10-L12 and https://github.com/ProfessionalWiki/AutomatedValues/blob/master/psalm.xml#L12-L15

There are also other Psalm failures about Wikibase classes not being found, dunno what is causing those. WikibaseExport runs checks with 38 without problems. Then again, you could switch Psalm and PHPStan back to older MW versions if they pass on those with Compat.php being excluded.

QuentinJanuel commented 1 year ago

Okay this should be working fine now, I have included MW38 in PHPUnit (sadly MW39 doesn't seem to work but I didn't investigate)

The workflows are running fine on my side image

If they don't for you, that's probably because of cache issues: I think your Psalm and PHPStan are running using MW37 despite the CI indicating MW38 because it is already cached. I cleared the cache and everything works fine for me.

JeroenDeDauw commented 1 year ago

Thank you @QuentinJanuel! We applied your changes via https://github.com/ProfessionalWiki/AutomatedValues/pull/28