Account created on 22 November 2010, about 14 years ago
#

Merge Requests

More

Recent comments

🇦🇺Australia acbramley

@jweowu sorry I missed that, feel free to contribute to the MR.

🇦🇺Australia acbramley

1 small nit otherwise looks good!

🇦🇺Australia acbramley

Thanks @nterbogt! Let us know if you need a hand with any of the other changes :)

🇦🇺Australia acbramley

I think 400 lines seems reasonable, I think the main change being that we're not judging by patch "size" since that metric is not readily available from an MR (AFAIK).

I wonder if we could find some examples of >400 line MRs that are considered too hard to review.

🇦🇺Australia acbramley

A simple gitlab CI file needs to be added too so tests run.

still needs actioning.

🇦🇺Australia acbramley

@flyke thanks for that, I was investigating how to remove this key myself for quite a while and came up short. The main issue I was running into is that when you save the node, the LayoutSectionItemList::equals was stopping the old data from being overwritten because toArray drops the key, therefore it doesn't save. However, since you're creating a new component (and therefore new UUID) it should work!

You also need to do this for every revision, otherwise you get the same errors on the revisions page as well.

🇦🇺Australia acbramley

It looks like a lot of the test failures are now something like this:

    1) /builds/issue/drupal-3426302/core/lib/Drupal/Core/Config/Schema/SchemaCheckTrait.php:211
    The 'status' setting for content blocks is deprecated in drupal:11.2.0 and is removed from drupal 12.0.0. It was unused, so there is no replacement. See https://www.drupal.org/node/3499836.

Which is coming from an update path test, I'm assuming because the db fixture has those keys set. Not entirely sure how we usually solve this kind of thing.

🇦🇺Australia acbramley

CI issues are being addressed in 📌 Test next major Active

🇦🇺Australia acbramley

HEAD is now failing since we're testing against D11 by default.

🇦🇺Australia acbramley

The PHPCS and PHPStan issues are unrelated to this issue.

🇦🇺Australia acbramley

acbramley changed the visibility of the branch 8.x-1.x to hidden.

🇦🇺Australia acbramley

acbramley changed the visibility of the branch 3495865- to hidden.

🇦🇺Australia acbramley

Applied #7 to an MR.

🇦🇺Australia acbramley

acbramley changed the visibility of the branch 3495867-deprecation-php-8.4 to hidden.

🇦🇺Australia acbramley

acbramley changed the visibility of the branch 8.x-1.x to hidden.

🇦🇺Australia acbramley

I would like to just get an opinion if removing if from updatedb can be done. Those other 2 calls don't really produce much valuable output and are much less likely to cause errors that are unknown.

🇦🇺Australia acbramley

acbramley made their first commit to this issue’s fork.

🇦🇺Australia acbramley

Can we get this in an MR, also the patch in #3 doesn't change the @messages placeholder in the string itself.

🇦🇺Australia acbramley

Sorry I found some more, should've done a more thorough search

🇦🇺Australia acbramley

acbramley changed the visibility of the branch 8.x-2.x to hidden.

🐛 | Diff | Fix CI
🇦🇺Australia acbramley

acbramley created an issue.

🇦🇺Australia acbramley

8.x-1.x is no longer accepting changes like this, only major bug fixes or security fixes.

🇦🇺Australia acbramley

Looking good, back to NW for tests

🇦🇺Australia acbramley

Back to green, would be good to get another review on this

🇦🇺Australia acbramley

I can't reproduce this with an Integer list. The error message shows there may be something wrong with your setup, integer lists use ListIntegerItem, not IntegerItem.

This plugin is only for list_string, list_integer, and list_float fields (as denoted in the plugin annotation) so I'm not sure how this is running on an integer field.

Can you post steps to reproduce from a fresh install?

🇦🇺Australia acbramley

This is actually already possible by configuring the plugin to use for this field, by default it's turned off.

Go to /admin/config/content/diff/fields and change the status field to use Core field diff.

🇦🇺Australia acbramley

This is quite a good improvement, I've left some minor comments and I think it would be good to get some tests too.

🇦🇺Australia acbramley

+1 this should be committed, we can eventually use Hook attributes in a class for this once the minimum version is 11.1.0

🇦🇺Australia acbramley

acbramley changed the visibility of the branch 3015152-support-third-party-10.3.x-php8.2 to hidden.

Production build 0.71.5 2024