Closed (fixed)
Project:
Simple social icons
Version:
8.x-3.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
23 May 2020 at 05:06 UTC
Updated:
17 Nov 2021 at 22:24 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
ankithashettyComment #3
ankithashettyComment #4
ankithashettyReplaced deprecated code to make it Drupal 9 ready in the following patch. Kindly review.
Thank you!
Comment #5
jaykandariOn it.
Comment #6
jaykandariHi @ankithashetty , Thanks for the patch.
Please find my findings below.
Patch apply fails both ways (via git & patch commands):
Upgrade Status reports OK
Only one issue core_version_requirement issue was reported and that is already being covered in the patch #4. Its just that patch apply needs to fix.
Also, there are some minor issues reported by Rector & drupal-check
drupal_set_message() issues are already being fixed..Once the patch applying is successful. I think remaining are just minor issues.
Comment #7
ankithashettyHello @JayKandari, thank you for reviewing the patch. I have updated a new patch as discussed. Please review the same.
Comment #8
jaykandariThanks @ankithashetty for the patch.
#7 applies cleanly now.
No issues Reported by Upgrade status module after applying patch.
Ran rector, No issues reported here also.
Thus marking it as RTBC.
Thanks !!
Comment #9
caspervoogt commentedPatch #7 left the
core: 8.xline intact, but that should not remain there. It should be replaced bycore_version_requirement: ^8 || ^9(it can't contain both lines). Working on a patch (ignore the one attached to this comment)Comment #10
caspervoogt commentedComment #11
caspervoogt commentedThis one (patch file. #10) applies cleanly for me. It removes the core line and adds a core_version_requirement line instead, since the info file cannot contain both.
Upgrade Status was not complaining about this module before this patch, but when I tried actually upgrading to 9 I kept getting "drupal/simple_social_icons 3.x-dev requires drupal/core ~8.0" errors. I was hoping my patch would solve that, but I think it will only be truly solved once this is committed, because I think when I run "composer require drupal/core:9.2.6" it actually checks the Simple Social Icons Git repo and the repo tells Composer this module requires Drupal 8, not 9.
The Drupal 9 deadline is just a month away. Can we get this committed please?
Comment #12
shelane