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
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#41 | interdiff.txt | 866 bytes | benjy |
#40 | 2684141-40.patch | 184.65 KB | benjy |
#38 | interdiff.txt | 6.82 KB | benjy |
#38 | 2684141-38.patch | 198.97 KB | benjy |
#36 | interdiff.txt | 592 bytes | benjy |
Comments
Comment #2
dawehnerLet's see where we are.
Comment #3
dawehnerSome fixes ...
Comment #7
dawehnerDoes this help?
Comment #9
dawehnerLet's see ...
Comment #11
benjy CreditAttribution: benjy at PreviousNext commented+1 from me, would love to see this.
Comment #12
dawehnerYeah, let's wait until the migration as plugin is in.
Comment #13
dawehnerThis has to wait until #2625696: Make migrations themselves plugins instead of config entities is in, as it will conflict quite a bit.
Comment #14
dawehnerReroll
Comment #17
benjy CreditAttribution: benjy at PreviousNext commentedNot sure what the CI error is?
Needs a use statement, there a few of these.
Comment #18
dawehnerUnassigning from myself. I'll work on it at sometime, unless someone else takes it.
Comment #20
benjy CreditAttribution: benjy at PreviousNext commentedReally keen to see this, running from PHPStorm is so convenient. Re-rolled and fixed the absolute namespaces at the same time.
Comment #22
benjy CreditAttribution: benjy at PreviousNext commentedFound the fatal, there was an empty test file. Also changed a few more tests over to KTB. Had some fails with EntityFileTest, not sure if that is specific to my environment or not, see what the bot says.
Comment #24
benjy CreditAttribution: benjy at PreviousNext commentedMore empty files.
Comment #26
benjy CreditAttribution: benjy at PreviousNext commentedSome more fixes, I reverted back EntityFileTest because that uses $base_url which doesn't seem to get set correctly in the new KTB.
Comment #28
benjy CreditAttribution: benjy at PreviousNext commentedDown to one fail, not sure how to get the vfs stuff working in MigrateFileTest
Comment #29
dawehner@benjy
Just to be clear, the old kernel test did the same stuff when setting up the request.
Comment #31
benjy CreditAttribution: benjy at PreviousNext commentedFixed both files tests using vfs, thanks to dawehner for the help getting public:// working.
Comment #33
dawehnerWait, is the idea really to remove the test?
Comment #34
benjy CreditAttribution: benjy at PreviousNext commentedhmm, that test passed locally for me, could be something to do with the global temp files on the bot.
Comment #35
benjy CreditAttribution: benjy at PreviousNext commentedNo, that was just a bad interdiff, I actually moved it. You can tell because its the one that failed in #31 :)
Comment #36
benjy CreditAttribution: benjy at PreviousNext commentedFixed.
Comment #37
dawehnerThanks a lot to take over this!
Nitpick: Let's use
$this->root
We missed to move those files
Comment #38
benjy CreditAttribution: benjy at PreviousNext commentedThanks for the review, pretty excited to get this in, the experience of running these locally is much better although its a shame the bot doesn't give better results.
Feedback done.
Comment #39
dawehnerWell, a) this will improve at some point and b) I just don't care that much as you need to run them locally anyway in order to fix stuff.
Comment #40
benjy CreditAttribution: benjy at PreviousNext commentedI went over the patch looks pretty good to me, every file seemed to have a namespace and a comment update along with the file being moved.
I've removed one assertion from the EntityFileTest because of a limitation with realpath and vfs which was causing it to fail. We have a similar assertion right below that tests the same thing without vfs anyway.
Comment #41
benjy CreditAttribution: benjy at PreviousNext commentedForgot the interdiff.
Comment #42
dawehnerThank you benjy!
Comment #43
alexpottCommitted 4478bb1 and pushed to 8.1.x. Thanks!
Fixed on commit