Account created on 4 August 2008, about 17 years ago
#

Merge Requests

More

Recent comments

🇮🇪Ireland lostcarpark

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

🇮🇪Ireland lostcarpark

Merged and moving to fixed.

🇮🇪Ireland lostcarpark

Thank you for working on this.

I think the test is pretty neat. It's nice that you were able to make it a kernel test rather than a Functional one.

I've run the automated tests manually, and run the automated tests on my local.

Moving to RTBC.

🇮🇪Ireland lostcarpark

Merged and moved to fixed.

🇮🇪Ireland lostcarpark

I have run the tests manually and manually tested, and I'm happy with this change.

I will open a follow-on issue to add a hook to delete the config.

🇮🇪Ireland lostcarpark

I was there!

🇮🇪Ireland lostcarpark

I think it's confusing that relative dates work on Datetime fields, but not on Timestamps, so it would be great to get this over the line.

🇮🇪Ireland lostcarpark

Update issue link.

🇮🇪Ireland lostcarpark

lostcarpark created an issue.

🇮🇪Ireland lostcarpark

I assumed that with no dependencies it would automatically install, but I see that's not happening.

I think that moving to config/install is the best solution. However, I think that will fail if the config is already present.

I think we should add an hook_uninstall to remove the config so that if the module is reinstalled it won't fail. However, I think this can be done in a follow on issue.

🇮🇪Ireland lostcarpark

Apologies.

I realise that starting a test in this change will offer several advantages.

  1. It will let us use "Test only changes" to verify the test fails without the change.
  2. It sets us up for extending the test in future changes.

Apologies to add to the requirements mid issue, but I think the test is quite small, and adding it in the issue has enough benefits to justify.

Thank you for your work on this.

🇮🇪Ireland lostcarpark

Some excellent ideas here.

I think the reason for making classes final might have been to stop PHPStan warning about "unsafe use of new static()" in the create function. I'm aiming to move to autowiring, which should allow most create functions to be deleted. But preventing subclassing is clearly not desired, so let's get the final removed.

Permissions sound like a good way of dealing with redirecting. I could see value in having "allow email verification", and "bypass verification" permissions for each verification form. If you don't want to allow user creation, just set "allow email verification" to "authenticated users". If you want to skip verification for logged in users, then set "bypass verification" to "authenticated users". Otherwise, "bypass verification" could be granted to a role for users who won't have to complete verification.

Another useful option would be to add an option to the verify email form settings to allow a user to be added to a role on successful verification. That way, users could be added to a "verified users" role once verified, and that role could be granted "bypass verification" so they wouldn't be challenged again.

Moving the redirect out of the form does make sense. I have a few thoughts on the best way to do it.

As there are a few different pieces, I'll create some smaller issues to tackle them individually.

🇮🇪Ireland lostcarpark

lostcarpark created an issue.

🇮🇪Ireland lostcarpark

lostcarpark created an issue.

🇮🇪Ireland lostcarpark

Change merged successfully. Thanks for working on this @ritarshi_chakraborty.

Please review the contribution record. If you would like to attribute credit to an organisation, you should select them there. If not, you should tick the "I am volunteering my time" box. You can also set a default for future contribution.

🇮🇪Ireland lostcarpark

I have reviewed and the change looks perfect. I've also run the tests locally and done some manual testing.

Moving to RTBC.

🇮🇪Ireland lostcarpark

Thank you for your work on this issue, @nidhi27.

Please review the contribution record. If you would like to attribute credit to an organisation, you should select them there. If not, you should tick the "I am volunteering my time" box. You can also set a default for future contribution.

🇮🇪Ireland lostcarpark

Thanks for your work on this issue, @souvik_banerjee.

Please review the contribution record. If you would like to attribute credit to an organisation, you should select them there. If not, you should tick the "I am volunteering my time" box. You can also set a default for future contribution.

🇮🇪Ireland lostcarpark

Merged successfully. Thanks for working on this.

Please review the contribution record. If you would like to attribute credit to an organisation, you should select them there. If not, you should tick the "I am volunteering my time" box. You can also set a default for future contribution.

🇮🇪Ireland lostcarpark

I have tested, and the config setting was created as expected.

I'm happy to move to RTBC.

🇮🇪Ireland lostcarpark

Thank you for your work on this.

Everything is technically correct. I would just like the aliases grouped with the service they are aliasing for clarity.

Please reorder, so that the service definition is followed by all aliases of that service. There is no need for a blank line between the service and its aliases, just between each distinct group.

Should be a simple change, then ready to merge.

🇮🇪Ireland lostcarpark

I fully agree, and I wasn't suggesting that should be a blocker.

🇮🇪Ireland lostcarpark

Makes sense.

Although, when I install on 8.4, I still get a bunch of deprecation warnings from contrib modules.

🇮🇪Ireland lostcarpark

lostcarpark created an issue.

🇮🇪Ireland lostcarpark

lostcarpark created an issue.

🇮🇪Ireland lostcarpark

Changed examples to assume PHP8, and using property promotion, as that should be the recommended pattern being promoted. Removed example showing a previous example rewritten using property promotion. Added a note saying that if using versions prior to PHP8, properties will need to be declared outside constructor.

🇮🇪Ireland lostcarpark

Moving to fixed.

🇮🇪Ireland lostcarpark

I reviewed the schema and it looks perfect. As there is no code using it yet, there is no valid test for this, and it does not impact the module.

🇮🇪Ireland lostcarpark

Moving to fixed.

🇮🇪Ireland lostcarpark

I've reviewed this change, and it looks perfect to me.

I've verified the tests pass, and I've done some local testing to confirm.

Change merged.

🇮🇪Ireland lostcarpark

Tests passing after rebase. Hopefully this can be merged into 11.2.x.

🇮🇪Ireland lostcarpark

Looks like 🐛 Initial workspace-published revisions are marked as new too late Needs review is now fixed. Going to attempt a rebase.

🇮🇪Ireland lostcarpark

lostcarpark created an issue.

🇮🇪Ireland lostcarpark

In Autowiring section, added "use" for Autowire attribute. Also removing no longer required use, and alphabetizing.

🇮🇪Ireland lostcarpark

I was there!

🇮🇪Ireland lostcarpark

Sorry, I got totally swamped with DrupalCon Vienna. I will review this today.

🇮🇪Ireland lostcarpark

lostcarpark created an issue.

🇮🇪Ireland lostcarpark

I had a feeling you did. :-)

🇮🇪Ireland lostcarpark

The default description uses the old PHP 7 syntax. Do we still need to include this? I think we should look at moving the PHP 8 property promotion to the top of the page, as that is now the recommended method.

🇮🇪Ireland lostcarpark

Add new section on autowiring dependency injection for forms.

🇮🇪Ireland lostcarpark

I would like to opt-in this module: https://www.drupal.org/project/advent_calendar
It has 12 issues, and all maintainers agree we are ready to try GitLab issues. We understand we cannot revert this change and there may be some unexpected issues as early adopters.

🇮🇪Ireland lostcarpark

Ah, that's from the first time contribution workshop, not from the mentor orientation.

🇮🇪Ireland lostcarpark

@igorgoncalves I presented the Wednesday session, linked below, though I have a strong suspicion you were at both.

https://www.drupal.org/project/mentoring/issues/3553661 📌 Mentor Orientation at DrupalCon Vienna 2025 - Wed 15th Oct 2025 10:30-11:15 Active

🇮🇪Ireland lostcarpark

lostcarpark created an issue.

🇮🇪Ireland lostcarpark

lostcarpark created an issue.

🇮🇪Ireland lostcarpark

lostcarpark created an issue.

🇮🇪Ireland lostcarpark

Thank you for your work on this. I've checked I can assign credit on core again, and looks good now.

🇮🇪Ireland lostcarpark

lostcarpark created an issue.

🇮🇪Ireland lostcarpark

lostcarpark created an issue.

🇮🇪Ireland lostcarpark

Thank you @xjm!

🇮🇪Ireland lostcarpark

lostcarpark created an issue.

🇮🇪Ireland lostcarpark

Reviewed and assigned credit.

📌 Mentor Meeting - 24 September 2025 Active feat: Mentor Meeting - 24 September 2025

By: chrisdarke
By: igorgoncalves
By: rachel_norfolk
By: dinarcon
By: volkswagenchick
By: rodrigoaguilera
By: alvarito75
By: lostcarpark
By: kristen pol
By: antojose
By: greggmarshall
By: Amarshall
By: salimlakhani
By: rfay

🇮🇪Ireland lostcarpark

Hope I've got everyone!

📌 Mentor Meeting - 1 October 2025 Active feat: Mentor Meeting - 1 October 2025

By: lostcarpark
By: igorgoncalves
By: rachel_norfolk
By: mradcliffe
By: greggmarshall
By: chrisdarke
By: Amarshall
By: rodrigoaguilera
By: salimlakhani
By: antojose
By: kristen pol
By: michaellenahan
By: markie
By: alvarito75
By: darren oh

🇮🇪Ireland lostcarpark

I have attributed contribution credit.

📌 Mentor Meeting - 8 October 2025 Active feat: Mentor Meeting - 8 October 2025

By: lostcarpark
By: igorgoncalves
By: rodrigoaguilera
By: tmarkie
By: mradcliffe
By: john cook
By: volkswagenchick
By: darren oh
By: salimlakhani
By: dinarcon
By: greggmarshall
By: alvarito75
By: Amarshall
By: rfay
By: kristen pol
By: anmolgoyal74
By: antojose
By: chrisdarke

🇮🇪Ireland lostcarpark

I was suspecting it wasn't just me. Hope it's not too painful to resolve.

🇮🇪Ireland lostcarpark

In fact I have lost it on all projects I maintain.

🇮🇪Ireland lostcarpark

Not sure if this is related, but I also seem to have lost the ability to grant issue credit on Mentor project (node 2476263).

🇮🇪Ireland lostcarpark

Adding attendance through new contribution record (the one downside of this is it's not as clear on the issue record).

🇮🇪Ireland lostcarpark

Oh hang on... I was able to earlier, but I seem to have lost it again.

🇮🇪Ireland lostcarpark

Yes, it's working for me now. Thanks!

🇮🇪Ireland lostcarpark

I believe @manuel-ranzmeir referenced in comment #13 is actually @ranzinator2000.

My lesson from this is to get everyone working on an issue to post their own comment, rather than get one person to type the usernames.

I have added the users to the Contribution record.

🇮🇪Ireland lostcarpark

This issue has been open for over a month with no objections.

A little over half of sites using this module are using the 3.x version, with just under half on 4.x.

Unfortunately, I don't think we can get statistics on how many are using on older versions of Drupal that aren't compatible with the 4.x branch.

Marking 3.x unsupported would give users an incentive to migrate to 4.x.

This change requires no code change, just removing 3.x from the supported modules on the module page.

I'd appreciate if someone could move to RTBC.

🇮🇪Ireland lostcarpark

I think someone verifying they can still install the module with this change in D10 or D11 should be sufficient, since all it is doing is removing D9 compatibility.

🇮🇪Ireland lostcarpark

I think @ranzinator2000 is the d.o username for @manuel-ranzmeir and have updated the contribution record accordingly. Please let me know if this is not correct!

Yes, that's correct. I think one of my learnings from this contribution day is to get each person helping with an issue to post a comment, rather than one comment with all the usernames.

🇮🇪Ireland lostcarpark

That's an interesting idea. I would expect there are very few sites running without views, because so many things depend on them. I would think if a site doesn't have Views enabled, it's a deliberate choice, and they won't appreciate an update to our little module enabling them. I would prefer a warning on the Site status page.

🇮🇪Ireland lostcarpark

Thank you! It's not super urgent, I just created the issue so it wouldn't get forgotten.

🇮🇪Ireland lostcarpark

lostcarpark created an issue.

🇮🇪Ireland lostcarpark

I have reviewed this change, and I believe it has been carried out according to Drupal coding standards.

I have verified the tests pass with this change.

As the change is exclusively to test code, I don't believe manual testing is relevant here.

I'm moving to RTBC.

🇮🇪Ireland lostcarpark

Removing igor from issue assignment.

🇮🇪Ireland lostcarpark

lostcarpark created an issue.

🇮🇪Ireland lostcarpark

I performed Novice Triage on this issue. I am leaving the Novice tag on this issue because removing user module as dependency of child classes is a straightforward novice task.

🇮🇪Ireland lostcarpark

I performed Novice Triage on this issue. I am leaving the Novice tag on this issue because I believe fixing the MR issue is good novice task.

🇮🇪Ireland lostcarpark

Marking issue complete as all child tasks closed.

🇮🇪Ireland lostcarpark

lostcarpark created an issue.

🇮🇪Ireland lostcarpark

This issue seems to be resolved by 🐛 Fix PHP 8.4 implicit nullable deprecation Needs review , so can probably be closed as a duplicate.

🇮🇪Ireland lostcarpark

I've tested the patch and it's resolved the deprecation warnings for me.

🇮🇪Ireland lostcarpark

I get the following when running drush updb on a site with pathauto enabled:

PHP Deprecated: Drupal\pathauto\Commands\PathautoCommands::generateAliases(): Implicitly marking parameter $types as nullable is deprecated, the explicit nullable type must be used instead in /web/modules/contrib/pathauto/src/Commands/PathautoCommands.php on line 89

PHP Deprecated: Drupal\pathauto\Commands\PathautoCommands::deleteAliases(): Implicitly marking parameter $types as nullable is deprecated, the explicit nullable type must be used instead in /web/modules/contrib/pathauto/src/Commands/PathautoCommands.php on line 135

PHP Deprecated: Drupal\pathauto\AliasStorageHelper::__construct(): Implicitly marking parameter $entity_type_manager as nullable is deprecated, the explicit nullable type must be used instead in /web/modules/contrib/pathauto/src/AliasStorageHelper.php on line 79

🇮🇪Ireland lostcarpark

I have added a before script to .gitlab-ci.yml to temporarily remove [Hook] and [LegacyHook] attributes.

🇮🇪Ireland lostcarpark

I'm happy with this, and tests are green so happy to go RTBC.

🇮🇪Ireland lostcarpark

This could be the same issue as 🐛 SID 2776211 contains div element Needs review , which failed if a "Rich text" text format wasn't configured.

It was fixed by moving the header out of the view, and into the Controller.

If that was the issue, this could probably be closed, but I agree making "Views" a dependency is a good idea.

🇮🇪Ireland lostcarpark

Added attendees to issue credit using new system.

Production build 0.71.5 2024