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.
Problem/Motivation
As title
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#13 | Screenshot 2020-07-23 at 11.05.39 AM.png | 32.35 KB | samiullah |
#11 | interdiff-9-11.txt | 823 bytes | jungle |
#11 | 3160020-11.patch | 10.04 KB | jungle |
Comments
Comment #2
jungleFirst iteration
Comment #3
jungleFor reviewer:
cd core && yarn && yarn spellcheck:core
to make sure spelling check passes.Comment #4
ultrabob CreditAttribution: ultrabob commentedI'll work on reviewing this.
Comment #5
ultrabob CreditAttribution: ultrabob commented<C3><BC>bersetzung
' to '<C3><9C>bersetzung
' for example. That seem unrelated to this patch)I think this one is good to go!
Comment #6
jungleThanks @ultrabob for reviewing.
Self-review:
$iids
was refactored to$ids
, but maybe changing it to$item_ids
is better. But$ids
is ok probably. So Stay RTBC.Comment #7
alexpottHere iids means item IDs. I think changing this to $item_ids is better because if we also have to deal with fids - ie. feed IDs then things are less confusing.
Comment #8
ravi.shankar CreditAttribution: ravi.shankar at OpenSense Labs commentedComment #9
ravi.shankar CreditAttribution: ravi.shankar at OpenSense Labs commentedHere I have addressed comment #7, please review.
Comment #10
alexpottLooking at it this is a bit odd. It could be
for quite a bit less code. Given we're changing every line I think this is okay.
For what it is worth the
items
property on the Feed entity needs work. It's dynamically declared, only has meaning whilst a feed is being updated. It's all a bit odd.Comment #11
jungle@alexpott, thanks for the review!
Addressing.
Out of scope there to rewrite the whole. Leaving it to a separate issue if necessary.
Comment #12
alexpottYeah I was not suggesting to tackle that here - definitely a separate issue.
Comment #13
samiullah CreditAttribution: samiullah at Salsa Digital commentedWe installed the fresh version of drupal 9.1
Applied the patch
Ran cd core && yarn && yarn spellcheck:core
No spelling errors were observed
@alexpott this can be moved to RTBC if code review part is fine
Comment #14
jungleThanks @samiullah, let's set back to RTBC.
BTW, you do not have to take a screenshot. To copy and paste the output is enough, wrapping with the
code
tag.Comment #15
alexpottCommitted 8e736dc and pushed to 9.1.x. Thanks!
Committed c689c8e and pushed to 9.0.x and cherry-picked to 8.9.x. Thanks!
As this is only test changes backported to 8.9.x for consistency.
I didn't created @samiullah since as @jungle says screenshots are not required.