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.
We need to port the D7 feeds_tamper HTML Entity Decode plugin to D8 tamper.
Comment | File | Size | Author |
---|---|---|---|
#14 | interdiff_11-14.txt | 1.19 KB | jamesdixon |
#14 | htmlentitydecode-2960830-14.patch | 2.52 KB | jamesdixon |
| |||
#11 | interdiff_7-11.txt | 2.11 KB | jamesdixon |
#11 | htmlentitydecode-2960830-11.patch | 2.1 KB | jamesdixon |
| |||
#8 | interdiff_5-7.txt | 403 bytes | jamesdixon |
Comments
Comment #2
jamesdixon CreditAttribution: jamesdixon commentedI've created a draft o the HTML Entity Decode plugin. Thanks for having a look.
Comment #3
jamesdixon CreditAttribution: jamesdixon commentedAgain, cleaning these plugins up based on @ericgsmith's earlier feedback. This makes them much simpler.
Comment #4
ericgsmith CreditAttribution: ericgsmith as a volunteer commentedThanks for this too - just some very minor changes - I'm happy to update the test to extend the new base class if you like.
Unused, can be removed.
As per the string to time review - I believe this can be one line.
Code standard for acronyms is to use CamelCase e.g. HtmlEntityDecode
Comment #5
jamesdixon CreditAttribution: jamesdixon at Dialed In Design commentedThanks for the feedback I've made these updates.
Comment #6
jamesdixon CreditAttribution: jamesdixon commentedComment #8
jamesdixon CreditAttribution: jamesdixon at Dialed In Design commentedWas missing a dependency in the last patch. Here's one that works!
Comment #9
ericgsmith CreditAttribution: ericgsmith as a volunteer commented@James the new patches look like there were all enhancements to 2, not 3.
The changes in 3 were good - any reason to revert them?
Comment #10
jamesdixon CreditAttribution: jamesdixon at Dialed In Design commentedNo sorry, I must've grabbed the wrong patch. I will reroll this patch with the stuff from 3 when I get a chance.
Comment #11
jamesdixon CreditAttribution: jamesdixon at Dialed In Design commentedThanks for catching that @ericgsmith! I have added #3 back in, and also added the new Tamper Base Test class.
Cool, interdiff 7 11!
Comment #12
ericgsmith CreditAttribution: ericgsmith as a volunteer commentedNice - nearly there with this one too :)
As per the string to time review, you can use $this->plugin here rather than creating a new plugin.
Would also be good to test for the exception that is thrown.
Comment #13
jamesdixon CreditAttribution: jamesdixon at Dialed In Design commentedSounds good. When testing for the exception, do you do a try catch statement assertment somehow, or just assert it is a string?
Comment #14
jamesdixon CreditAttribution: jamesdixon at Dialed In Design commentedI think I'm picking up what you're throwing down @ericgsmith. I think this patch nails that exception confirming assertion you're looking for in the test.
Comment #15
ericgsmith CreditAttribution: ericgsmith as a volunteer commentedAwesome stuff - this looks good to go
Comment #17
ericgsmith CreditAttribution: ericgsmith as a volunteer commented