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.From #1818560-31: Convert taxonomy entities to the new Entity Field API:
+++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Tests/EfqTest.phpundefined @@ -69,6 +69,6 @@ function testTaxonomyEfq() { - $this->assertEqual($term->tid, $tid, 'Taxonomy term can be created based on the IDs'); + $this->assertEqual($term->id(), $tid, 'Taxonomy term can be created based on the IDs');maybe a novice follow-up to look for other asserts where the punctuation is missing from sentences.
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | 1965510-14.patch | 19.9 KB | TR |
| #14 | diff.txt | 792 bytes | TR |
| #13 | diff.txt | 1.78 KB | TR |
| #12 | 1965510-12.patch | 19.9 KB | DyanneNova |
| #11 | 1965510-11.patch | 19.9 KB | TR |
Comments
Comment #1
shanethehat CreditAttribution: shanethehat commentedMissing punctuation and descriptions added for all taxonomy test assertions.
Comment #3
Wim LeersI'm somewhat glad the first patch contains a syntax error, because committing this before #1818560: Convert taxonomy entities to the new Entity Field API would require another reroll there. This issue should be postponed on #1818560: Convert taxonomy entities to the new Entity Field API, sorry for not making that more explicit :(
Comment #4
shanethehat CreditAttribution: shanethehat commentedEvery embarrassing typo has a silver lining :)
Here it is again without the typo, even if it's not useful right now.
Comment #5
Wim LeersThank you! :)
If you won't, then I'll reroll your patch once that other patch is committed. Thanks again!
Comment #6
TR CreditAttribution: TR commentedNow that #1818560: Convert taxonomy entities to the new Entity Field API is committed (almost 6 months ago!), it's time to consider this patch again.
I re-rolled the patch in #4 against the current D8 HEAD. I also corrected some spelling problems that were present in the previous patch. Let's see what the testbot says ...
Comment #8
TR CreditAttribution: TR commentedSorry, syntax error. That's embarrassing. This one should be correct.
Comment #10
TR CreditAttribution: TR commentedOK, let me work on this. I understand the testbot warnings, those are my fault. I don't immediately understand the failures, those are in sections of the patch I just copied. I'll see what I can do to make this run green.
Comment #11
TR CreditAttribution: TR commentedOK, I fixed the failures in the original patch and fixed the warning that I introduced. Tests now run green locally, which is something I should have tested before I posted the patch. Let's try this now on the Drupal QA server.
Comment #12
DyanneNovaEverything looks good to me except for a few typos (landcode in place of langcode). I've added a patch for those.
Comment #13
TR CreditAttribution: TR commentedYup, missed those. Thanks for the correction. Here's the interdiff so people can see the differences between #11 and #12. Diff shows there's still a typo in there, so I'll post another patch shortly.
Comment #14
TR CreditAttribution: TR commentedOK, here's the new patch, and interdiff between #12 and #14 (this one).
Comment #15
DyanneNovaI looked over everything again and don't see any more typos, so this should be good to go.
Comment #16
Xano14: 1965510-14.patch queued for re-testing.
Comment #17
Xano14: 1965510-14.patch queued for re-testing.
Comment #18
webchickNice clean-up. Thanks!
Committed and pushed to 8.x.