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.
Added a test, asserting the link is there.
Comment | File | Size | Author |
---|---|---|---|
#10 | taxonomy-test-569076-9.patch | 1.04 KB | wamilton |
#6 | taxonomy-test-569076-6.patch | 1.04 KB | drupal_was_my_past |
#4 | taxonomy-test-569076-4.patch | 1.01 KB | drupal_was_my_past |
#2 | taxonomy-test-569076-2.patch | 1011 bytes | drupal_was_my_past |
taxonomy-test-breadcrumbs.patch | 1.5 KB | alonpeer | |
Comments
Comment #1
cburschkaPlease don't leave commented-out lines of code lying around.
I'm on crack. Are you, too?
Comment #2
drupal_was_my_past CreditAttribution: drupal_was_my_past commentedRe-roll patch from #1.
Comment #3
xjmThanks for your work on this patch. One minor correction for the doxygen:
This should begin "Tests" rather than "Test". Reference: http://drupal.org/node/1354#functions
Also note that the Drupal 8.x patch will need to be rerolled, because the core directory structure for Drupal 8 has now changed. (For more information, see #22336: Move all core Drupal files under a /core folder to improve usability and upgrades). When the patch has been rerolled, please set the issue back to "Needs Review."
Tagging as novice for the task of rerolling the Drupal 8.x patch with the above doxygen fix.
Comment #4
drupal_was_my_past CreditAttribution: drupal_was_my_past commented@xjm Got it. Thanks for the feedback. Here's the re-rolled patch.
Also, I noticed that there are plenty of other docblocks that aren't of the right tense (e.g. says "Test" instead of "Tests"). Should I create a new issue for all of those in this file?
Comment #5
xjmThanks @rocket_nova! Regarding the other docblocks with the wrong tense, they will be cleaned up in #1326644: Clean up API docs for taxonomy module.
Edit: I was about to RTBC, but then I noticed the wordings are a bit wonky in the comments:
Here the juxtaposition of "to" and "from" is confusing. I'd change this to:
Looks like the word "as" is missing. I think it should be:
And I'd just rewrite this as:
Let me know if you're up to reroll again with these changes. Thanks!
Comment #6
drupal_was_my_past CreditAttribution: drupal_was_my_past commentedThanks @xjm! Here's the re-rolled patch with your suggestions.
Comment #7
xjm#6: taxonomy-test-569076-6.patch queued for re-testing.
Comment #9
xjmHm, looks like we need a reroll here. Once it's rerolled I think this is RTBC. Sorry for letting this one slip by me earlier. :)
Comment #10
wamilton CreditAttribution: wamilton commentedpickaxe is BUSTED!
git log -Staxonomy_get_term_by_name
Patch applied fine for me after applying to that commit. Committed and rebased for the attached.
Comment #11
wamilton CreditAttribution: wamilton commentedComment #12
xjmThanks wamilton!
Comment #13
Dries CreditAttribution: Dries commentedThis patch looks good.
Was it a follow-up from another issue?
Comment #14
webchickNice find. Committed and pushed to 8.x and 7.x. Thanks!