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.
This was an annoying glitch in the aggregator API.
Other functions go aggregator_[verb]_[thing being verbed], but for whatever reason load doesn't. Let's fix that.
Comment | File | Size | Author |
---|---|---|---|
#9 | aggregator-load-order-4.patch | 1.95 KB | kathyh |
#7 | aggregator-load-order-3.patch | 2.47 KB | kathyh |
#6 | aggregator-load-order-3.patch | 2.47 KB | kathyh |
#5 | aggregator-load-order-2.patch | 2.47 KB | kathyh |
aggregator-load-order.patch | 2.85 KB | webchick | |
Comments
Comment #2
webchickMeh. I don't actually care enough to fix whatever's causing test failures myself, but I bet it's something stupid.
Tagging "novice".
Comment #3
kathyh CreditAttribution: kathyh commentedAssigned to kathyh. Will take a look.
Comment #4
kathyh CreditAttribution: kathyh commentedNovice notes:
GET ISSUE
-Select Issue from queue that is tagged with "Novice":
http://drupal.org/project/issues/search/drupal?issue_tags=Novice
-Assigned it to myself
IF IT COMES WITH A PATCH - APPLY PATCH
-downloaded new *AMP stack to get the php version for 8.x and then installed d8 site
-downloaded greasemonkey to better REVIEW the patch in the issue http://drupal.org/project/dreditor
-get a copy of drupal8 from git:
git clone http://git.drupal.org/project/drupal.git drupal8 (where drupal8 is the dir for the repo)
git checkout -b local 8.x
-to use the patch (from http://drupal.org/patch) downloaded cygwin
-install patch module and utils/cygutils (to get the unix2dos utility)
with the patch module to run: patch -p1 < patchfile
FIX THE ISSUE
-now that I have the patch, looking at test dir in the aggregator dir to see which test fail and why (e.g. fixing the issue)
-ask questions in #drupal on IRC http://webchat.freenode.net/
-ask questions in #drupal-contribute - see http://groups.drupal.org/node/175539
CREATE THE NEW PATCH
-after done will look at Making a Drupal Patch with GIT http://drupal.org/node/707484 - run unix2dos on files
git config --global core.autocrlf true
git config --global core.safecrlf true
git diff > aggregator-load-order-3.patch
UPLOAD PATCH AND TEST
-switch the status to "needs review" and upload the patch. This will wake up the test-bot... eventually.
-if it passes, then leave status as "need review" and wait for RTBC
Comment #5
kathyh CreditAttribution: kathyh commented-function aggregator_load_feed removed from patch (left as *_load), uploading for test
Comment #6
kathyh CreditAttribution: kathyh commented-re-upload (status was ignored)
Comment #7
kathyh CreditAttribution: kathyh commentedok - google says I need to change status to needs review - let's see if this calls the test bot
Comment #8
marvil07 CreditAttribution: marvil07 commented@kathyh: Thanks for providing the patch!
There are 3 functions that match
/aggregator_*_load/
:aggregator_feed_load()
,aggregator_category_load()
andaggregator_feed_items_load()
.The problem with renaming the first two functions is that they both are menu wildcard loaders, and sadly there is not a way to set manually the callback name for them, so they should not be removed.
Another possibility is to declare the right-named functions, and there call the menu wildcard loader functions manually, but that seems even more confuse to me.
@kathyh: Based on the reasoning above, this hunk need to be removed, and after it, I guess this issue will be RTBC ;-)
Comment #9
kathyh CreditAttribution: kathyh commented@marvil07 - thanks for the feedback and help in #drupal - much appreciated.
Change was to remove menu wildcard loaders from the patch. Resubmitting.
Comment #10
marvil07 CreditAttribution: marvil07 commentedAs mentioned on #8, IMHO this is ready now :-)
Comment #11
Dries CreditAttribution: Dries commentedCommitted to 8.x. Thanks.
Comment #12
Eric_A CreditAttribution: Eric_A commentedThe log indicates that this was committed to D8 and D7. The D7 commit must have been accidental.
Comment #13
Eric_A CreditAttribution: Eric_A commentedComment #14
webchickOh dear. :)
Rolled back in 7.x. Thanks for escalating, Eric_A, and thanks for the patch kathyh!!
Comment #15
Tor Arne Thune CreditAttribution: Tor Arne Thune commentedThis needs a change notification, no?
Comment #16
pillarsdotnet CreditAttribution: pillarsdotnet commented@Tor Arne Thune:
Yup. Follow your own link and create one; it's not hard. ;-)
Comment #17
Tor Arne Thune CreditAttribution: Tor Arne Thune commentedWell, seeing as the issue is marked Novice, I was hoping to give a hint to someone stumbling upon the issue that has never done it before.
Comment #18
webchickComment #19
rickmanelius CreditAttribution: rickmanelius commentedFor your review (this is my first change notice submitted... so let me know if I mucked something up).
http://drupal.org/node/1295398
Comment #20
marvil07 CreditAttribution: marvil07 commentedIt look fine :-), maybe(because target is developers for this change notice) mentioning that some functions matching the pattern mentioned on this issue title end up not changing because they are menu wildcard loaders(see comment 8 for more information) is relevant. Otherwise I guess it's ok, but I'm not an expert on change notices ;-)
Comment #21
pillarsdotnet CreditAttribution: pillarsdotnet commentedRewrote the change notice. Only one function was renamed, although three were considered.
Also posted comments to 7.x and 8.x API pages.
Comment #22
marcingy CreditAttribution: marcingy commentedReadding change notice part of issue title so people realise that is all that is outstanding
Comment #23
pillarsdotnet CreditAttribution: pillarsdotnet commentedHuh? No; the change notice has been written.
Comment #25
xjm