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 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 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 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 commentedSorry, syntax error. That's embarrassing. This one should be correct.
Comment #10
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 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 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 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.