Follow up for "Adding Assertions to Drupal - Test Tools"

Thanks @smustgrave and The Kind Folks @#needs-review-queue-initiative

Rerolled in a new MR against 10.1.x.

mallezie → closed merge request !1179

Thanks! Totally forgot about this one.

Just checked the new MR, and seems the same to the original. (Which i did, so probably i'm not the one allowed to put it in RTBC).

Closed the old merge request.

Thanks for the turnaround

ON Drupal 10.1.x
Did a quick search for https://www.drupal.org/node/2408013 → and found 4 instances
Applied the MR and they were addressed.

Tagging for framework manager review since this caused its share of debate back in the day.

Am I just unlucky, or is every issue I worked on lately needing policy updates/[Important-People] review?

@Spokje I think that means you are working on important things. :)

fwiw these all look like good changes to me - they're notifying of developer errors, which is the right place for an assert.

The only one that had me scratching my head was saving an entity without validation, since the exception potentially prevents bad data being saved, but that also looks like a situation you can only get into via developer error so the assertion should still catch it before it actually happens on production anywhere.

@Spokje I think that means you are working on important things. :)

To stupidly stumble where no person has gone before!

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

_Very_ unsure if this is allowed, but since this was a reroll and not done by myself, I'm going t put this back to RTBC per #21 📌 Follow up for "Adding Assertions to Drupal - Test Tools" Fixed

Agreed we should do this, there are probably more examples but we can gradually open issues for those when we spot them. Untagging for framework manager review.

Committed e434d61 and pushed to 10.1.x. Thanks!

Also to confirm you're definitely allowed to re-RTBC something that's only been re-rolled since it was last RTBC.

Thanks @catch, there's probably a very clear policy out there, but my search-fu failed me on this one.
Very handy to know for the future.

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

Spokje → closed merge request !3281

Adjusting issue credit as credit is not provided for button-click rebases without additional contribution.

This fix caused issue with PHP 8.3 compatibility #3375693-14: Fix deprecated assert_options() function usage for PHP 8.3 →

Let's polish assertions there