That sounds like a feature request for Pantheon (if their workarounds are not sufficient). Acquia and Platform support setting env variables, as do things like .htaccess and nginx config.
Like I said, DotEnv is really made for local development, so all this talk of web hosting demonstrates how people will be confused if this goes in. If people really want this feature, its a composer require away.
I could easily see us taking the base added here and doing a lot of special things in follow-ups. Like injecting the DB credentials for the primary/default DB automatically. Or providing an env variable override process for settings and config. But the base logic/support would be needed first.
I'm totally on board with settings.php honoring env variables. IMO thats the first part to do, not the last.
What I have seen is that .env files are helpful for local development, and less so for staging and prod servers which have other means of writing env variables. Given that we have standardized on DDEV for local dev, which already support setting env variables from config or .env files, do we really need this? The OP was written in an age before Docker based local dev.
It seems like the additional tests are not coming quickly. Would maintainers consider fixing the bug without tests? Otherwise we are forcing everyone to live with a bug because we are worried that the bug might return one day.
This module is no longer needed in 10.3+ https://www.drupal.org/project/drupal/issues/144538 🐛 User logout is vulnerable to CSRF Fixed
mass.gov was experiencing this issue as well. We worked around it by setting max-age=0 on the tabs block. See https://drupal.slack.com/archives/C1BMUQ9U6/p1715802644024269 for lots more discussion.
I mean, release branches wont get pipelines but thats not such a big deal. Projects that care can override the rules. IMO this can go in as is.
I think that one is OK because $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH will pass.
The Nightly scheduled pipeline at https://gitlab.com/drupalspoons/devel/-/pipeline_schedules is not running due to this.
moshe weitzman → created an issue.
This went in with https://git.drupalcode.org/project/domain_perm/-/merge_requests/2
moshe weitzman → created an issue.
We will be abandoning 1.x branch in favor of 2.x
Done in 2.x branch
See comment in MR.
moshe weitzman → made their first commit to this issue’s fork.
In my example I said "makes a new policy".
Your example is generally about the drawbacks of altering services from other modules.
We have already settled that concern with permitted, but not recommended (see italicized warning) →
. final takes this warning a lot further. Your example is a perfectly reasonable thing for a site to do, and the final makes it harder. Site developers don't need to care about multiple cooperating access policies.
Many cooperating policies is a rare use case. We've learned this with node access grants system - we built a system which handles multiple cooperating policies but few understand the system. We would benefit from a simpler and less cooperative system here IMO. Sorry I'm going off topic here.
IMO, this comes down to how paternal we want our code to be. I slightly prefer less paternal.
as soon as one module chooses to extend over decorating, the gate is immediately closed for all other modules to do the same.
If custom or contrib code makes a new policy and chooses to extend another (the original policy could be active or inactive - both valid use cases), why would all other active policies cease to work as designed? I must be missing something.
I dont feel too strongly about this. The OP makes some good arguments for this. Some counter-arguments:
- Its a bit unrealistic to have multiple access policies magically work together. Each site would have different requirements about what happens when the policies overlap
- Is decorating a class really much safer than extending it. You still "depend" on the inner class in a significant way.
- I'm from the "we're all consenting adults" philosophy. I would slap @internal on the policy classes and then any developer who chooses to extend does so at own risk.
I agree that applyTo() is a gotcha. The merge or the Exception sound good to me.
so the value in _PHPUNIT_EXTRA needs to be variable to $PHPUNIT_VERSION (is that possible?)
Yes, we do that in the templates. For example https://git.drupalcode.org/project/gitlab_templates/-/blob/main/includes...
We now have \Drupal\Core\Database\Database::commitAllOnShutdown. Should we keep this issue open or change it somehow?
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.
Merged a fix and released new version of DTT
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.
Thank you.
moshe weitzman → made their first commit to this issue’s fork.
This would be helpful for Drush's PmCommands to avoid phpstan complaining when accessing $extension->origin
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...
The generator works with Drush11-. I will soon convert it to Drush 12.
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...
moshe weitzman → created an issue.
Can this be closed?
I disabled CSPELL. Other projects can be the testbed for that job.
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.
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.
Gábor Hojtsy → credited moshe weitzman → .
Correct, I refer to the output files not being written and thus not being browseable/downloadable.
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.
Would be relatively easy automation thats generic so a google search yields example code:
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?
Improve Description. Now using Full HTML unfortunately
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.
hestenet → credited moshe weitzman → .
hestenet → credited moshe weitzman → .
hestenet → credited moshe weitzman → .
hestenet → credited moshe weitzman → .
hestenet → credited moshe weitzman → .
hestenet → credited moshe weitzman → .
hestenet → credited moshe weitzman → .
I added a section to the templates docs https://www.drupal.org/node/3356364/revisions/view/13323370/13351385 → . Ready to announce IMO.
Added section about Gitlab pages auto-publish
Why is a wait needed in order to view the h1 text? That should be returned in the initial response, right?
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...
mcdruid → credited moshe weitzman → .
Fixed branch condition check
This is great. My only worry is that few people will discover it. Oh, well, lets start here.
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
moshe weitzman → made their first commit to this issue’s fork.
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.
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.
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.
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.
To clarify, the prior comment describes a workaround. This is still an active feature request that is close to a bug IMO.
Follows the pattern of https://www.drupal.org/project/drupal/issues/3398321 📌 Optimize GitLab resource requests phase 1 Fixed (drupal core)
moshe weitzman → created an issue.
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.
Please lets not hold up useful doc changes for trivial improvements.
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).
+1 from me (the current user maintainer).
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 →
Added note about before_script and after_script
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)
e0ipso → credited moshe weitzman → .
Looks to me like this is done in latest version.
Agree with #8. This is Wont Fix or By Design.
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.
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.
Add link to https://github.com/ddev/ddev-drupal-contrib
The redirect controller could forward unknown issue ids to the gitlab url, without any mapping. That way there is one url to check.
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.
keycdn no longer has Nightwatch tests so that looks like a bug.
fjgarlin → credited moshe weitzman → .
The error was my fault. Pipeline succeeded.
I used this is KeyCDN and pipeline succeeds and is using correct php version https://git.drupalcode.org/project/keycdn/-/pipelines/43386
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.
Code LGTM. I am getting an error when trying to run the downstream pipeline "An error occurred while making the request."
Documented new matrix testing env vars
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.
moshe weitzman → created an issue.
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.