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

Merge Requests

More

Recent comments

πŸ‡ΊπŸ‡Έ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

Fixed branch condition check

πŸ‡ΊπŸ‡Έ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

This was already implied by us linking to MongoDB which uses this technique. But I have now made it explicit. https://www.drupal.org/node/3356364/revisions/view/13301062/13302270 β†’

πŸ‡ΊπŸ‡Έ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

The error was my fault. Pipeline succeeded.

πŸ‡ΊπŸ‡Έ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

πŸ‡ΊπŸ‡ΈUnited States moshe weitzman Boston, MA

moshe weitzman β†’ created an issue.

πŸ‡ΊπŸ‡ΈUnited States moshe weitzman Boston, MA

I think we should merge this. I dont know how long it will take to do the gitlab_templates change. I'm not in charge there anymore.

πŸ‡ΊπŸ‡ΈUnited States moshe weitzman Boston, MA

We can remove our script override when https://git.drupalcode.org/project/gitlab_templates/-/merge_requests/60 gets in.

πŸ‡ΊπŸ‡ΈUnited States moshe weitzman Boston, MA

moshe weitzman β†’ created an issue.

πŸ‡ΊπŸ‡ΈUnited States moshe weitzman Boston, MA

One thing that this patch doesn't handle (I think) is that editing is sometimes done on a different domain than viewing (for security). For example, the author is editing on edit.mass.gov but pastes in a link to www.mass.gov/node/n. Ideally this code would recognize www.mass.gov as an additional base_url and this recognize the pasted link as internal. So, give sites a way to provide a list of domains, not just the one that the author is using.

πŸ‡ΊπŸ‡ΈUnited States moshe weitzman Boston, MA

Are you sure about that? The intent is to allow any 10.x version. See https://getcomposer.org/doc/articles/versions.md#caret-version-range-

πŸ‡ΊπŸ‡ΈUnited States moshe weitzman Boston, MA

This patch no longer applies to the most recent release (Sep 30) :(

πŸ‡ΊπŸ‡ΈUnited States moshe weitzman Boston, MA

Thanks for giving us some better vocab. IMO, virtually no contrib module needs CONCURRENCY. Thats why I favor straight phpunit for the default implementation. Core and a few contrib projects can override from there.

πŸ‡ΊπŸ‡ΈUnited States moshe weitzman Boston, MA

The 5.x branch provides Drupal 10 compat. We were unable to find a way to do that in the 4.x branch because of dependency hell between D9, D10 and PHP. See πŸ“Œ Drupal 10 compatibility Fixed . If you can find a way in 4.x, we can delete the 5.x branch.

I suggest asking a question when you dont know something. It can be quite alarming for users when a maintainer claims that a supported release is in fact a fork within the official repo. I dont think bringing D10 compat qualifies as a fork.

πŸ‡ΊπŸ‡ΈUnited States moshe weitzman Boston, MA

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

πŸ‡ΊπŸ‡ΈUnited States moshe weitzman Boston, MA

jQuery UI Draggable no longer a dependency

πŸ‡ΊπŸ‡ΈUnited States moshe weitzman Boston, MA

jQuery UI Draggable no longer a dependency

πŸ‡ΊπŸ‡ΈUnited States moshe weitzman Boston, MA

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

πŸ‡ΊπŸ‡ΈUnited States moshe weitzman Boston, MA

Added docs for usiing https://github.com/cweagans/composer-patches in project's composer.json.

πŸ‡ΊπŸ‡ΈUnited States moshe weitzman Boston, MA

I agree. This can happen during a database restore, for example. I made a contrib module that resolves this problem until this issue is discussed and developed.

πŸ‡ΊπŸ‡ΈUnited States moshe weitzman Boston, MA

When CONCURRENCY is used, are the tests running with sqlite regardless of the DB that the user specified. Or is just a results table thats using sqlite? If its the former, we should error out if the specified DB was mysql or postgres IMO.

Also, we are now not honoring _PHPUNIT_EXTRA, and making unavailable all the CLI options that are used with it (--fail-on-warning, --filter, --exclude-group, ...).

IMO we should start with core overriding in run-tests.sh. If we can make that generally useful for contrib projects, it belongs in gitlab_templates. My .02

πŸ‡ΊπŸ‡ΈUnited States moshe weitzman Boston, MA

In my instance, that check is preventing the values from being unset, not unsetting them. Changing status after my review.

Production build https://api.contrib.social 0.61.6-2-g546bc20