Going through the module for other issues, I found two possible improvements for the code.
They are probably a bit too large to just be committed without issue or discussion (which I do for typos), but I don't think they warrant separate issues or a larger discussion.

CommentFileSizeAuthor
#5 2957650-5--minor_changes.patch2.78 KBdrunken monkey
#5 2957650-5--minor_changes--interdiff.txt661 bytesdrunken monkey
#2 2957650-2--minor_changes.patch2.75 KBdrunken monkey

Comments

drunken monkey created an issue. See original summary.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Active » Needs review
StatusFileSize
new 2957650-2--minor_changes.patch2.75 KB

Patch attached, reviews are welcome.

Status: Needs review » Needs work

The last submitted patch, 2: 2957650-2--minor_changes.patch, failed testing. View results

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented 
--- a/src/Entity/Index.php
+++ b/src/Entity/Index.php

The changes here look very good!

It looks like some changes broke a couple of tests.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs work » Needs review
StatusFileSize
new 2957650-5--minor_changes--interdiff.txt661 bytes
new 2957650-5--minor_changes.patch2.78 KB

Huh, OK, good that I created the issue … ^^"
All the warning were in HEAD and fixed with #2930839: Update tests according to latest PHPUnit compatibility changes, but the BackendTest fails are really caused by this. And after a quick look it's clear, why: the new ExampleContentTrait::getItemIds() code didn't preserve the order of passed IDs anymore. The attached revision (sadly not as clean as before) should fix this.

  • drunken monkey committed 45fb0a1 on 8.x-1.x 
    Issue #2957650 by drunken monkey, borisson_: Fixed two minor problems.
drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs review » Fixed

OK, tests are green, I guess that's enough.
Committed.
Thanks again for reviewing, Joris!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.