🇬🇧United Kingdom @rachel_norfolk

No worries!

Honestly, I want to get a release out as much as you.

There are four or five issues that have been touched recently - either at Needs Work or Needs Review. Moving those through to RTBC will be key to getting that release out.

updating statuses...

Thank you all!

Okay, I’m happy with this now!

I’m guessing Bohemia isn’t actually working on this right now so unassigning - anyone can go right ahead and resolve the merge error!

Can we resolve the merge error, please? I think this may be close...

Actually, we need this against the 3.x branch, please.

If someone can create a Merge Request, someone else test it, then I will happily merge.

When testing, please consider the scenario that someone has both a base path and a translation.

Thanks cgmonroe!

We have a rather awkward combination of both a patch and a branch containing a fix.

Can we ensure that the work in the branch is converted into a Merge Request as that is the thing that I will merge, once it is confirmed it contains what is required for this issue (and only this issue).

Thanks

This doesn’t look like it is RTBC any more. Maybe someone can confirm where we are?

Thanks @jrockowitz for adding me!

rachel_norfolk → created an issue.

The patch does what we expect it to do - a warning is logged that there was an alias in the table that couldn’t be matched.

For the purposes of this issue, I think that makes it RTBC.

It might be that a future issue looks at ways of ensuring the table is clean.

Please don’t berate the maintainers on an issue, sachbearbeiter, it simply doesn’t help the situation in any way.

Maintainers are all humans doing an essential role voluntarily and their inclination to do that is only harmed by that type of comment.

May I suggest instead that you contribute to the issues here that still need to reach a reviewed state and help them be ready for a possible release?

However, if you are so disappointed in the contribution culture in Drupal, may I suggest trying another cms, instead?

The wording is more accurate, thanks!

To be honest, as I look through various core modules, well-regarded contrib modules, and even the examples module, the wording in this location isn’t really all that consistent. Your version is as good as anything, though.

rachel_norfolk → made their first commit to this issue’s fork.

Thanks very much, avpaderno!

And also thanks Vishal.kadam! You are right that the language could be better and I’d be delighted if you could add an issue to the project along those lines so someone could work on it. I’d love to credit you for spotting it.

updating typo in title

I wonder if we could get some of the above RTBC committed to allow a decision to be made on a release?

Heh - that was a weird hour of staring at an MR wondering why the tests were not running automatically!

Turns out, the tests folder was incorrectly named test. All fixed but needs a review now by someone other than n-m-daz and me.

The test will fail right now. That is expected as noted above.

So, I’m looking at the patch attached and it is trying to update the function views_url_alias_node_node_update() which no longer exists in 3.x

I don’t think the problem as described still exists in 3.x and, therefore, can be closed. However, I’ll set it to Postponed (needs more info) for the time being and maybe ueshiy can enlighten as to whether they see the problem in 3.x (I don’t, for what it is worth).

If not, I suggest it is set to Closed (outdated).

Time works for me and the MR looks good!

This is a great idea!

The edited paragraph might need some thought, though, as the topic of development tools is less likely to be related to the core gates.

Okay, I believe this is easy for review.

rachel_norfolk → created an issue.

Made an RC1 release

well, I think it is time to make a release...

Looks good to me, too.

Thanks everyone!

rachel_norfolk → made their first commit to this issue’s fork.

I agree that we need some tests. I also agree that creating those tests shouldn’t hold up an initial release of the project, it can come later.

updating list...

Awesome work, everyone!

I suppose having tests that fail against 3.x now but start passing once that other change request is merged will be a rather appropriate test ;-)

@nicholass - I was going on the message in comment #6. There’s certainly a belief that the issue is fixed in @dev, but in a different way.

Purely pressing the Create Merge Request button so it is easier to see what we have and it can be reviewed...

updating status...

(annoyingly, this only happens on save atm)

updating status...

Heh - I’m slightly amused that it took me longer to remember how to make a test site with a base path than it did to actually test the proposed change!

With the change applied, the test site correctly takes into account the base path. It also continues to work when there isn’t a base path.

There is an update function to update aliases that might already be stored, which is also nice.

I think this is a good one. RTBC

Setting to maintainer needs more info as that’s kinda what we are looking for.

The project homepage does say that the module only works on content entities that extend Drupal\Core\Entity\ContentEntityInterface so this might be out of scope at this time.

just adding [duplicate] to title so I don’t keep looking in here for things to do...

Added comment to say we don’t need to drop support for ^8.8 at this stage - unless there is something I’ve missed?

rachel_norfolk → changed the visibility of the branch project-update-bot-only to hidden.

feels like needs work...

Nice work, everyone!

Looks good to me, too!

Just updating the assignment. In general, we tend to just have people write a comment saying they are working on something and for how long. That way, it avoids it getting left assigned (and I used to be massively guilty of this!)

rachel_norfolk → created an issue.

wanna have a go at it?

Added the wording suggested above - it does help!

This is a great piece of work! I’m beginning to worry I’ve pitched some training I have organised for you a bit low! :-D

I’ve added a couple of comments that I think will make it more maintainable.

I like the direction, though.

I was just coming here to say how to test and you beat me to it!!

Thanks for the review.

Nice one!

I also slightly changed the version requirement to at least 10.1 as Change Record says that is required for that method of calling the hook in a class.

I think you still need to press the button in Gitlab to tell it to create the Merge Request.

Before you do, though, one thing worth bearing in mind is that, whilst inside a .module t() is acceptable, once in a Hook class, we really should use the StringTranslationTrait to add in $this->t() and use that.

You can see an example in core/modules/block/src/Hook/BlockHooks.php

Also, I’m still getting used to reading these new hook styles so I might have missed something!

Needs a review

rachel_norfolk → created an issue.

Okay, you have me intrigued as to what the performance concerns might be.

There’s currently a hook_form_alter that checks to see whether the form class has a particular method. See:

$form_object = $form_state->getFormObject();
if (method_exists($form_object, 'getEditableConfigNames')) {
  //Add a message.
}

Plus checking for a couple of other specific form classes that we know would alter config but work slightly differently.

Do you think inspecting the class would be intensive?

rachel_norfolk → created an issue.

rachel_norfolk → created an issue.

rachel_norfolk → created an issue.

Really nice work all - you are now Drupal contributors!!

I’ve madde a couple of tiny modifications but I’m loving what you have written!

I’ve (just) added the Config Warning → contribution module but I definitely see this as “backfill” for something that should really be in core.

rachel_norfolk → created an issue.

I’ve started to add content to the docs so it helps people know how to add things and shared with mentoring-leads.

Enjoyed the meeting tonight!

• There are messages displayed if config is overriden in the settings.php file (so that site builders don't get confused why the config change in the GUI doesn't have an effect). That should work together somehow, and look consistent.
  -- yes, I would suggest the code sits alongside that, too.
• If something is in config ignore, then it also shouldn't show up - or show up with a different message?
  -- very good point! Should be a reasonably easy check to make, I expect
• This shouldn't show up on a development site because it would drive site builder crazy. May a toggle to turn it off, that can be overridden in a local settings.php file?
  -- yeah, I was thinking we could have a option in the development section of admin that is turned off by default - a site owner only turns it on for prod sites, if they want.
• The wording of the message: This is also relevant for projects that don't have a dev team - as long as there is a separate copy of the site for development and exporting config. Something like "This configuration should better be changed in the development environment and then exported. Otherwise they will be reverted back during the next config import."
• And maybe... and option for an extra field for whom to contact if this really needs changing on prod to ensure it doesn't get lost?
  -- yeah - another option in the development area, I would suggest, though I guess the whole text could be overidden just like any text can be overridden in the site.

The code above actually works, Tony. It inspects what base classes the form is implementing and can make a prediction of whether it will affect running config from that. This means it even works on contrib modules!

mradcliffe → credited rachel_norfolk → .

updating vienna names

mradcliffe → credited rachel_norfolk → .

rachel_norfolk → created an issue.

Note: BlockListBuilder extends ConfigEntityListBuilder and that ’s probably worth looking out for, rather than the special case of blocks.

Playing around with this a little and the following working “pseudo code” (i.e. please don’t actually use this as is - it is just to test ideas) gives the idea of what I’m thinking:

function xx_form_alter(&$form, FormStateInterface $form_state, $form_id) :void {

  // Get out of here if it is not an admin form.
  if (!\Drupal::service('router.admin_context')->isAdminRoute()) {
    return;
  }

  // We need an actual object rather than just a jumble of array items.
  $form_object = $form_state->getFormObject();

  // Assume forms have no config to begin with.
  $has_config = FALSE;

  // Does the getEditableConfigNames method exist on the form?
  if (method_exists($form_object, 'getEditableConfigNames')) {
    $has_config = TRUE;
  }

  // Are we editing an entity that has config form for it's setup?
  if ($form_object instanceof EntityForm) {
    $entity = $form_object->getEntity();
    if ($entity instanceof ConfigEntityInterface && !$entity->isNew()) {
      $has_config = TRUE;
    }
  }

  // Permisions being permissions.
  if ($form_object instanceof UserPermissionsForm) {
    $has_config = TRUE;
  }

  // Blocks being blocks.
  if ($form_object instanceof BlockListBuilder) {
    $has_config = TRUE;
  }
  // Okay, warn people.
  if ($has_config) {
    \Drupal::service('messenger')->addWarning(t("This form is likely to alter the running configuration of the site and, therefore, changes should probably go through the development quality processes."));
  }

}

Adding an option for the warning to include a custom message.

Adding link to Config Override Warn as inspiration

rachel_norfolk → created an issue.

changes needed
- add step to set status of new cloned issue to Active

rachel_norfolk → created an issue.

Thank you all!

I tell you what, lostcarpark, let me run this meeting to test the script above, then you can test any changes we make next month?

adding contributors

Following the notes for the meeting next week as testing...

just editing slightly as I think we said it was me looking at GitLag, not Chris?

rachel_norfolk → created an issue.

adding that spreadsheet structure to get us started... https://docs.google.com/spreadsheets/d/1WdwP4fFr2k2ADOTWB59w1evjdf6djAdB...

rachel_norfolk → created an issue.

Just a note that this change would be an absolute gift to those who like to game the credit system

I do love the seemingly random way component is set as we move issues between projects ;-)

As we are happy as a team to progress this, it seems time to move into the core project so someone can make an MR.

mradcliffe → credited rachel_norfolk → .

Just formatting the tag correctly so it gets counted along with all the others.