Closed thomas-topway-it closed 1 year ago
Merging #727 (af32d04) into master (ad5ea25) will not change coverage. The diff coverage is
n/a
.
@@ Coverage Diff @@
## master #727 +/- ##
=========================================
Coverage 39.31% 39.31%
Complexity 2046 2046
=========================================
Files 75 75
Lines 7233 7233
=========================================
Hits 2844 2844
Misses 4389 4389
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
To me this looks like the embedded jquery has been patched. This would break again on jquery update.
What is the alternative?
... well, the original source https://github.com/Wilto/Dynamic-Carousel isn't maintained anyway. So risk of breaking again is almost 0. An alternative would be to use an alternate plugin that is maintained while ideally keeping the SMW interface unchanged :-)
Yes, I see. Well then the scope of this PR is clear: to fix the bug when bootstrap is present. Maybe we should open another issue that SRF is using an outdated plugin.
I think this is fine (and roughly what I had in mind in https://github.com/ProfessionalWiki/chameleon/issues/346#issuecomment-1249013722).
The upstream plugin is dead and if there happens to be an update there then the diff will at least show the name changed again and git blame should trace it back to this issue.
... if you look at the timestamps, most of the js has not been touched the last years. As long as they work, it's fine. But fixing security issues or keeping things running with more recent versions of other components is another story. Having an additional issue to follow-up on this one is definitely the right direction.
To my opinion, this PR can be merged to get your original problem solved.
fix issue https://github.com/SemanticMediaWiki/SemanticResultFormats/issues/724 (carousel not working)