- Status changed to Needs review
almost 2 years ago 9:06am 21 January 2023 - 🇪🇨Ecuador jwilson3
@devad Thanks.
I'm not familiar with Ludwig so had a look around, and at the issue here: 🌱 [LIST] Ludwig integrations in contrib modules Active to see what other modules are doing.
The first one I looked at in detail was SMTP: #3137440: SMTP - Ludwig integration → and it required a follow-up issue to put the letter "v" before the version string here: #3177608: Ludwig.json file fix →
I noticed the patch on this issue does not have a "v", so basically two requests:
1.- just want to make sure the patch is coded correctly and the "v" is not necessary.
2.- Link to some instructions on how to properly test these patches for all the maintainers who are unaware of Ludwig-based sites to quickly get on board and test the code that they'll have to maintain. - Status changed to Needs work
almost 2 years ago 8:40pm 22 January 2023 - Status changed to Needs review
almost 2 years ago 9:05pm 22 January 2023 - 🇭🇷Croatia devad
Hi. Thanks for reply.
The letter "v" before the version string is not needed any more. The latest few Ludwig versions do not need it.
To test the Ludwig integration... apply the patch to your project and then follow the Installation and Usage → Ludwig tutorial. If all libraries required by your module are reported as "Installed" and your module works as expected without the "Class not found" errors that's sufficient to confirm that Ludwig integration is successful.
- 🇨🇦Canada mandclu
The documentation page shows the "v" before the version strings as well, but documentation can certainly become outdated too. Is there some reference we can see to verify that this is no longer required?
- 🇭🇷Croatia devad
Re #7.
it is not Ludwig standard any more to prefix version numbers with "v" since all non-numeric characters inside the version strings are gracefully ignored now. There is no harm if you do add the extra "v" prefix to the version number... but there is no need or recommendation for that any more. This part of Ludwig code (PackageManager.php file) explains why.
// Two versions of the same package are not possible. // If multiple providers require the same package // we keep the highest required version only, since it has // the best probability to work for all providers, and // it is the most secure. And we mark all lower package // versions as 'Overriden'. $loop1_packages = $packages; $loop2_packages = $packages; foreach ($loop1_packages as $loop1_name => $loop1_package) { foreach ($loop2_packages as $loop2_name => $loop2_package) { // Let's strip all non-numeric characters from the package // versions in order to compare them successfully. if (($loop2_package['name'] == $loop1_package['name']) && ($loop2_name != $loop1_name) && (preg_replace("/[^0-9.]/", "", $loop2_package['version']) < preg_replace("/[^0-9.]/", "", $loop1_package['version']))) { $packages[$loop2_name]['status'] = 'Overridden'; } } }
- Status changed to Fixed
almost 2 years ago 6:34pm 19 February 2023 - 🇨🇦Canada mandclu
I don't love the idea that this file specifies the patch release, as it would seem to mean we become responsible for keeping that up to date. That said, it looks like the library has been pretty stable for the last year or so. Merged in.
- 🇭🇷Croatia devad
Great. Thank you @mandclu
I have added the SVG Image Field module to Ludwig ecosystem (list of projects with active Ludwig integration).