Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.| Comment | File | Size | Author |
|---|---|---|---|
| #10 | drupal8.dep-tags.10.patch | 917 bytes | sun |
| #7 | drupal-2204143-dep-tags-7.patch | 9.21 KB | ianthomas_uk |
| drupal8.dep-tags.0.patch | 10.1 KB | sun | |
Comments
Comment #1
sundrupal8.dep-tags.0.patch queued for re-testing.
Comment #2
dawehnerThis is pretty straightforward.
Comment #3
webchickWe seem to be missing a change record about this? https://drupal.org/list-changes/published?keywords_description=drupal_im...
Comment #4
sunThe change notice should have been created for #2003568: Convert tags,attributes, diff and url validation unit tests to phpunit already...
Change notice: https://drupal.org/node/2212099
Comment #5
catchPlease split this into two issues/patches - that reduces the chance of cross-commits with other patches in the queue. And if they do cross-commit, only the 'removal' patch needs to be reverted + re-rolled instead of the whole thing.
Comment #6
sun@catch: Not sure I'm able to follow in this case — as visible in this patch, those tag explosion/implosion utility functions are used by 4 core modules only, they are barely used in contrib, and I don't think the amount of consumers is going to radically change anytime soon...?
Therefore, splitting this patch into conversions-only vs. removal-only doesn't make sense to me. If the conversions happen to cause conflicts, then it makes no difference whether the full patch or just the conversions are reverted.
Comment #7
ianthomas_uk@sun I think it's more about having a consistent policy so people know what to do when they are creating patches. I agree this patch is simple so is unlikely to cause problems.
Here's a version that keeps the functions themselves.
Comment #8
sunThanks! I still disagree with the split and would recommend to commit #0, but not my decision.
Comment #9
catchYes that's really it. If it's a not-really-api function in an optional module then it's less of an issue, but it takes a lot longer to discuss each individual case how much the function is used than just to roll the extra very simple patch, and HEAD getting broken from cross-commits is not a theoretical issue, has happened several times.
Committed/pushed to 8.x, thanks!
Comment #10
sunNothing broke or conflicted, so here's the second part again.
Comment #11
ianthomas_ukComfirmed there are no remaining calls or references to these functions in core. This is already fully covered by a change record: https://drupal.org/node/2212099
Comment #12
webchickCommitted and pushed to 8.x. Thanks!