Boston, MA
Account created on 31 March 2001, about 23 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States moshe weitzman Boston, MA

We now have \Drupal\Core\Database\Database::commitAllOnShutdown. Should we keep this issue open or change it somehow?

🇺🇸United States moshe weitzman Boston, MA

Do folks think this could cause pages to show the same tabs to one node? This is happenning on mass.gov and on several other sites. See https://www.drupal.org/forum/support/module-development-and-code-questio...

assert() is non-functional in Prod so I dont think that approach is as helpful.

🇺🇸United States moshe weitzman Boston, MA

Merged a fix and released new version of DTT

🇺🇸United States moshe weitzman Boston, MA

Configurable via class property sounds good. IMO we should default to fast, as we do today. Sites that vary the container_yamls will want to change that property.

🇺🇸United States moshe weitzman Boston, MA

moshe weitzman made their first commit to this issue’s fork.

🇺🇸United States moshe weitzman Boston, MA

This would be helpful for Drush's PmCommands to avoid phpstan complaining when accessing $extension->origin

🇺🇸United States moshe weitzman Boston, MA

DTT partially tests this. Maybe someone can expand the test using a query like you have to verify that cleanupEntities is getting processed. See https://git.drupalcode.org/project/dtt/-/blob/2.x/tests/Entity/UserCreat...

🇺🇸United States moshe weitzman Boston, MA

The generator works with Drush11-. I will soon convert it to Drush 12.

🇺🇸United States moshe weitzman Boston, MA

You managed to fix isues for standards that we dont use in this project. https://git.drupalcode.org/project/keycdn/-/blob/8.x-1.x/phpcs.xml?ref_t...

🇺🇸United States moshe weitzman Boston, MA

I disabled CSPELL. Other projects can be the testbed for that job.

🇺🇸United States moshe weitzman Boston, MA

Drush core has happily outsourced codebase assembly and audit to Composer. This makes Drush more maintainable. It is wins like these that have helped Drush thrive for more than 15 years.

I think efforts are better spent on getting Composer to work as you want. See #32. If you don't care to help issue that along, then use the composer extensions mentioned in #29, or make a contrib Drush command.

🇺🇸United States moshe weitzman Boston, MA

I'm pretty sure that it is by design that composer audit only considers SAs. Merely becoming unsupported is out of scope. You could make your own composer or drush command that considers the support status of a module.

🇺🇸United States moshe weitzman Boston, MA

Correct, I refer to the output files not being written and thus not being browseable/downloadable.

🇺🇸United States moshe weitzman Boston, MA

The briefer of those links is missing its artifacts even when using the download/browse button. Thats not by design, right? It sounded like you expected that only in the log output.

🇺🇸United States moshe weitzman Boston, MA

format change

🇺🇸United States moshe weitzman Boston, MA

IMO patch is the right word. Diff means its a change, but patch means its a change AND its meant to be applied to the contrib project, just like patches that have traditionally been uploaded to d.o. So the meaning here matches traditional patches on d.o. IMO.

What makes you think that this patch is a confusing word?

🇺🇸United States moshe weitzman Boston, MA

Improve Description. Now using Full HTML unfortunately

🇺🇸United States moshe weitzman Boston, MA

Each time we extract code into a reference we make it harder to just read and understand whats going on. I'm not necessarily opposed to this MR, but how often will modules need to override this? And how bad is it if they override the rules section when they do. I might prioritize understandability in this case and forego the references.

Sorry to mention this now after a lot of work has happenned.

🇺🇸United States moshe weitzman Boston, MA

Added section about Gitlab pages auto-publish

🇺🇸United States moshe weitzman Boston, MA

Why is a wait needed in order to view the h1 text? That should be returned in the initial response, right?

🇺🇸United States moshe weitzman Boston, MA

1. I'm not sure when that would happen. If a user is using the issue fork system, they get handed a branch name by the d.o. integration. That branch is non-default.

2. I dont think we need to worry about this now. This job will only run for the sites that have Pages, and even then it is very fast. If a project is doing something unusual with an extension, they can adjust accordingly.

FYI, workflow:rules still apply for this job - https://git.drupalcode.org/project/gitlab_templates/-/blob/1.0.x/include...

🇺🇸United States moshe weitzman Boston, MA

This is great. My only worry is that few people will discover it. Oh, well, lets start here.

🇺🇸United States moshe weitzman Boston, MA

I opened an MR. All a project has to do is create an mkdocs.yml and a docs/index.md. See https://git.drupalcode.org/project/keycdn/-/merge_requests/19. Material for MKDocs has great docs at https://squidfunk.github.io/mkdocs-material/. Of course any project can override this job to publish a different site.

You can see that this new CI job named pages succeeded by looking at the KeyCDN downstream pipeline attached to this MR. An example https://git.drupalcode.org/project/keycdn/-/jobs/423715

🇺🇸United States moshe weitzman Boston, MA

moshe weitzman made their first commit to this issue’s fork.

🇺🇸United States moshe weitzman Boston, MA

Thanks for testing. Its true that we currently discover commands src/Drush. However the documented place to put them is src/Drush/Commands. See "Drush 12 expects all commandfiles in the /Drush/ directory" at https://www.drush.org/12.x/commands/. So for optimum future compat, you would move them as this issue suggests. In practice its not likely to matter.

🇺🇸United States moshe weitzman Boston, MA

Thanks for the fix.

FYI I just changed Devel's create() method so it avoids this sort of problem and avoids a PHPStan complaint as well - https://gitlab.com/drupalspoons/devel/-/merge_requests/164.

🇺🇸United States moshe weitzman Boston, MA

As an aside, all these same issues apply to gitlab_templates too. There we want our cake and eat it too. We want sites to automatically pickup changes to the templates but occasionally we break stuff, either by mistake or on purpose.

🇺🇸United States moshe weitzman Boston, MA

Devel also broke with this change and it is not listed in your report so not sure the report is super accurate - https://gitlab.com/drupalspoons/devel/-/issues/483. Devel code is also on git.drupal.org so it is eligible.

🇺🇸United States moshe weitzman Boston, MA

To clarify, the prior comment describes a workaround. This is still an active feature request that is close to a bug IMO.

🇺🇸United States moshe weitzman Boston, MA

Follows the pattern of https://www.drupal.org/project/drupal/issues/3398321 📌 Optimize GitLab resource requests phase 1 Fixed (drupal core)

🇺🇸United States moshe weitzman Boston, MA

For me this is wont fix. Not worth maintaining the code to work around the issue. This is how templates get cluterred and hard to maintain.

🇺🇸United States moshe weitzman Boston, MA

Please lets not hold up useful doc changes for trivial improvements.

🇺🇸United States moshe weitzman Boston, MA

Is this failing the test when deprecations happen, or just reporting them? IMO deprecations are advisory only and should not result in a phpunit job failure by default. Also see https://drupal.slack.com/archives/C51GNJG91/p1698355817936279 for a discussion of better presentation of deprecations in Gitlab UI (move to Code Quality tab).

🇺🇸United States moshe weitzman Boston, MA

+1 from me (the current user maintainer).

🇺🇸United States moshe weitzman Boston, MA

Added note about before_script and after_script

🇺🇸United States moshe weitzman Boston, MA

Unforeseen needs arise. Breaks happen. Mistakes happen. Its OK to say "try" IMO.

You have other ways to add extensions if you wish, like using own Dockerfile to extend the web image. I'm experimenting with that for memcache module - https://git.drupalcode.org/project/memcache/-/merge_requests/18/diffs#64...

Minimalist docs for adding php extensions are in our our Overrride section https://www.drupal.org/node/3356364#s-overriding-parts-of-the-template (item 6)

🇺🇸United States moshe weitzman Boston, MA

Looks to me like this is done in latest version.

🇺🇸United States moshe weitzman Boston, MA

Got further. We are now testing with a real memcached. I'm seeing some Heisen failures remaining. The tests sometimes fail locally and sometimes succeed, often changing when I enable or disable xdebug. Perhaps we are not clearing data sufficiently between test runs. Maybe someone else can take a look.

🇺🇸United States moshe weitzman Boston, MA

I got further. I'm working in 📌 Add optional DDEV dev environment Needs review since it is easier to debug failures with a DDEV envronment. Lets commit that one or come back here later if we dont want to merge DDEV config.

🇺🇸United States moshe weitzman Boston, MA

The redirect controller could forward unknown issue ids to the gitlab url, without any mapping. That way there is one url to check.

🇺🇸United States moshe weitzman Boston, MA

The thing is that each module maintainer controls her composer.json and they are going to be too slow to add and maintain these script definitions. So i made a plugin to do that, and to assemble the codebase using same approach as Gitlab CI.

Of course other approaches are possible. I'm not sure what we specifically can do on this issue.

🇺🇸United States moshe weitzman Boston, MA

keycdn no longer has Nightwatch tests so that looks like a bug.

🇺🇸United States moshe weitzman Boston, MA

I developed https://github.com/ddev/ddev-drupal-contrib for this purpose. Folks are welcome to add it to various docs. It is looking like DDEV will become an official/default den env for drupal so this may be all we need.

🇺🇸United States moshe weitzman Boston, MA

Code LGTM. I am getting an error when trying to run the downstream pipeline "An error occurred while making the request."

🇺🇸United States moshe weitzman Boston, MA

Documented new matrix testing env vars

🇺🇸United States moshe weitzman Boston, MA

This issue. It will be added when someone is motivated to make an MR and shepherd it through to completion. So far nobody has turned up to do that.

🇺🇸United States moshe weitzman Boston, MA

The phpcs fails have been happenning for a long time in DrupalCI. This job is marked by d.o. as allow_failure: true so it doesn't block development. I suggest we fix this in some other ticket.

🇺🇸United States moshe weitzman Boston, MA

We already have arbitrary bash which doesnt even start a pipeline if false. This is keeps noise out of the UI. See https://git.drupalcode.org/project/gitlab_templates/-/blob/1.0.x/include...

🇺🇸United States moshe weitzman Boston, MA

The Gitlab UI can show on a PR basis how the number of failures went up or down. Maybe we need to look more at code quality reports for phpcs if we arent getting that feature today.

🇺🇸United States moshe weitzman Boston, MA

I think this can go in as is. I think the right solution for php version is for CORE_PHP_MIN to be defined dynamically based on the value of TARGET_CORE. That way a d9 run would have 7.4 as value.

🇺🇸United States moshe weitzman Boston, MA

LGTM. I think this needs input from config validation maintainers as well (leaving tag in for this).

🇺🇸United States moshe weitzman Boston, MA

This looks good as far as it goes. It doesnt handle any PHP version variation. For example, if keyCDN got rid of its matrix in favor of this, all testing would move to PHP8. Not sure if thats a worth holding up this MR or not.

🇺🇸United States moshe weitzman Boston, MA

This project has no test coverage whatsoever. So yes anything “would be nice”

🇺🇸United States moshe weitzman Boston, MA

One issue its that its hard to find the exact errors until you find the code quality tab.

  1. Add expose_as: code-quality to the artifact section. Then the artifact becomes browseable on a job page (e.g. https://git.drupalcode.org/project/keycdn/-/jobs/216272) like it is on the Composer job page.
  2. Is there any way to have teh errors in the console output? Lots of people will look there.
🇺🇸United States moshe weitzman Boston, MA

All variables link

🇺🇸United States moshe weitzman Boston, MA

I added example env variables in the scheduled pipelines docs https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr...

🇺🇸United States moshe weitzman Boston, MA

Fix tags

🇺🇸United States moshe weitzman Boston, MA

Add examples for scheduled pipeline variables

🇺🇸United States moshe weitzman Boston, MA

Thanks for linking to keycdn. Its my impression that keycdn approach is working, and is the recommended way. Lots of projects have already copied it. In what way is it undesireable?

🇺🇸United States moshe weitzman Boston, MA

Removed D9 testing. Cleaned console output.

🇺🇸United States moshe weitzman Boston, MA

I no longer see MemcacheBackendUnitTest in 8.x version. Did we lose test coverage over the years?

🇺🇸United States moshe weitzman Boston, MA

Fixed the DB issue. The sole remaining issue is a build failure on D9+Relay. Oddly, D10+Relay is passing.

I can turn on PHPCS if you wish. It only gives warnings and never fails the build, by default.

🇺🇸United States moshe weitzman Boston, MA

I'm not sure why there is a DB error. Why are we installing a custom mariadb service? Help wanted.

Dont the clients need some custom php extension and such? I dont see that in the old template so have not dealt with that in this MR.

🇺🇸United States moshe weitzman Boston, MA

moshe weitzman created an issue.

🇺🇸United States moshe weitzman Boston, MA

Add link to Mongo

🇺🇸United States moshe weitzman Boston, MA

Green :)

In addition, you might want to use Pipelines Schedules in Gitlab UI to setup a weekly run of D9 and any other permutations that are important to test (i.e. PHP version). The default for PRs is to test against Drupal 10.1.x-dev

Production build 0.67.2 2024