UK
Account created on 23 June 2006, about 19 years ago
#

Merge Requests

Recent comments

🇬🇧United Kingdom rachel_norfolk UK

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 ;-)

🇬🇧United Kingdom rachel_norfolk UK

@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.

🇬🇧United Kingdom rachel_norfolk UK

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

🇬🇧United Kingdom rachel_norfolk UK

updating status...

(annoyingly, this only happens on save atm)

🇬🇧United Kingdom rachel_norfolk UK

updating status...

🇬🇧United Kingdom rachel_norfolk UK

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

🇬🇧United Kingdom rachel_norfolk UK

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

🇬🇧United Kingdom rachel_norfolk UK

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.

🇬🇧United Kingdom rachel_norfolk UK

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

🇬🇧United Kingdom rachel_norfolk UK

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

🇬🇧United Kingdom rachel_norfolk UK

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

🇬🇧United Kingdom rachel_norfolk UK

Nice work, everyone!

🇬🇧United Kingdom rachel_norfolk UK

Looks good to me, too!

🇬🇧United Kingdom rachel_norfolk UK

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!)

🇬🇧United Kingdom rachel_norfolk UK

Added the wording suggested above - it does help!

🇬🇧United Kingdom rachel_norfolk UK

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.

🇬🇧United Kingdom rachel_norfolk UK

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

Thanks for the review.

🇬🇧United Kingdom rachel_norfolk UK

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.

🇬🇧United Kingdom rachel_norfolk UK

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!

🇬🇧United Kingdom rachel_norfolk UK

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?

🇬🇧United Kingdom rachel_norfolk UK

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

🇬🇧United Kingdom rachel_norfolk UK

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

🇬🇧United Kingdom rachel_norfolk UK

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

🇬🇧United Kingdom rachel_norfolk UK

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

🇬🇧United Kingdom rachel_norfolk UK

Enjoyed the meeting tonight!

🇬🇧United Kingdom rachel_norfolk UK
  • 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.
🇬🇧United Kingdom rachel_norfolk UK

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!

🇬🇧United Kingdom rachel_norfolk UK

updating vienna names

🇬🇧United Kingdom rachel_norfolk UK

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

🇬🇧United Kingdom rachel_norfolk UK

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."));
  }

}
🇬🇧United Kingdom rachel_norfolk UK

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

🇬🇧United Kingdom rachel_norfolk UK

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

🇬🇧United Kingdom rachel_norfolk UK

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?

🇬🇧United Kingdom rachel_norfolk UK

adding contributors

🇬🇧United Kingdom rachel_norfolk UK

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

🇬🇧United Kingdom rachel_norfolk UK

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

🇬🇧United Kingdom rachel_norfolk UK

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

🇬🇧United Kingdom rachel_norfolk UK

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

🇬🇧United Kingdom rachel_norfolk UK

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.

🇬🇧United Kingdom rachel_norfolk UK

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

🇬🇧United Kingdom rachel_norfolk UK

I would be delighted to add James to our list. He is bringing exactly what makes mentoring successful.

🇬🇧United Kingdom rachel_norfolk UK

Who authored the node? I can’t remember.

🇬🇧United Kingdom rachel_norfolk UK

Hi - did you mean to post this here? This is not a support issue queue.

I suggest you ask the question on the Drupal Slack Drupal.org/slack

🇬🇧United Kingdom rachel_norfolk UK

I hope you don’t mind me saying, sarveetsingh, but that is a really well written test comment. Contains everything it should. Nice work!

🇬🇧United Kingdom rachel_norfolk UK

Adding credit for Ángela Saldaña Contreras who made the logo actually work for us!!!

🇬🇧United Kingdom rachel_norfolk UK

That’s good news, sali38, and much appreciated!

🇬🇧United Kingdom rachel_norfolk UK

For the record, there seem to be a number of other related projects with the same restriction claimed.

🇬🇧United Kingdom rachel_norfolk UK

They will quite possibly have a Bash CLI. Especially if they are a new user, though, it may not be clear at all that they need to use it.

🇬🇧United Kingdom rachel_norfolk UK

Did a little chasing and slashrsm very kindly pressed all the right buttons. Thank you Janez!

Urgh - as I’m not a maintainer, I can’t give them the credit they deserve...

🇬🇧United Kingdom rachel_norfolk UK

Gonna contact a couple of the maintainers and see what we can rustle up..

🇬🇧United Kingdom rachel_norfolk UK

Events insurance purchased via www.events-insurance.co.uk. Insurance certificate in the Venue folder on google drive.

🇬🇧United Kingdom rachel_norfolk UK

Feels like this is complete - after all, drupalcampengland.org

🇬🇧United Kingdom rachel_norfolk UK

Okay, let’s get this moving…

On February 14th, we will update the list according to these rules:

  • If you have positively indicated you will take an active part i the running of the core mentoring program and have participated in running meetings etc in the last nine months, you stay in the list
  • If you have not positively indicated you intend to do so, we will remove you from the list and say thank you for all of the work you have done to this point.

Thanks

🇬🇧United Kingdom rachel_norfolk UK

As it happens, I’ve archived the OpenCollective entities, rather than deleted them. Maybe we will use them in the future.

🇬🇧United Kingdom rachel_norfolk UK

I guess we no longer need this now we have the CIC.

I’ll shut down the OC bits and pieces and close this ticket now, I think.

Production build 0.71.5 2024