Still getting a feel for this stuff with autowiring. I think this is fine, especially as PHPStan will pick it up.
If an entity type has all the right features enabled (interfaces, annotation, routeing etc) I think we can just enable it automatically.
For instance, Scheduled Transitions → is a project I manage which automatically, and only, allows features for entity types if it has the right features. E.g all the interfaces and things required for Workflows and Content Moderation
But it would make sense to have some kind of switch regardless, either for a entity type or bundle.
I think this can be fleshed out as this is developed, however.
My target would be getting the Entity Test entities work.
@marcoscano are you asking if there are other content entities that live standalone, just like a node?
I've worked on many projects, and can say there are many many instances of this.
If you'd accept the idea, I'll see if I can find some project time for it.
Rewording according to #4
Changes can be found in v4.3.4 and later.
The code is looking less insane with this revision.
The pill markup adds quite large diffsize. Willing to do away with it and stick to a simplified version if we can keep the markup and layout/flexbox pieces, so we can potentially use the markup+selectors in extended/custom themes.
This issue caused a break in 🐛 PageBlockDisplayVariant::__sleep() is no longer compatible with parent signature from ctools leading to PHP fatals RTBC . Mind reviewing that @japerry, since you're also a maintainer there.
Concise title.
Critical since it completely borks a site.
Proposed changes seem fine.
A screenshot is not necessary.
Please do not spam tags. See [#3156530]
FYI tests are green over at ✨ Improve failure messaging Needs review . Notably had to remove core 11 testing due to entity usage.
Found issues with existing tests
- Tests now use permission for 🐛 There is no way to delete file entities of other users Fixed (10.1 and later)
- Entity usage updated to disable automatic tracking, so we have full control of the tracking values.
- Cspell
- disabled 11.x testing for now, until Entity Usage supports it
dpi → made their first commit to this issue’s fork.
Deferring to @mstrelan + @acbramley as they are more active with Linkycosystem than I.
Does this mean we could deprecate the existing lazy proxy creation script
A worthwhile cause <3
We already support the nice thing, \Symfony\Component\DependencyInjection\Attribute\AutowireServiceClosure. AFAICT.
!service_closure should be avoided in new code.
File was deleted?!
That's an example of Pareto principal. Just a few commands
This is exactly what I was thinking. Well put.
One thing I'd like see in this implementation is supporting command discovery in vendor directory. So you would be able bundle console commands to a Composer package (not Drupal module).
I've tried to address this in MR Notes ✨ CLI entry point in Drupal Core (Dex) Needs review . You can absolutely set up a command that exists in vendor (via services yml, etc). However autodiscovery is limited to the same rules as Drupal, i.e. only in module directories. Until Drupal has the ability to discover modules/plugins/etc I dont think it makes sense for this issue/initiate or related issues like ✨ Directory based automatic service creation Needs review to go out of their way and look in vendor. Its a large can of worms that doesnt need a decision now; we can easily change it in the future.
I've built a POC a few years ago.
Neat, I'm glad we've evolved even further since then. We're in such a nice spot in 2024.
Cleaning up markup
The intricacies of how to match versions in Composer with Drupal build system needs work, otherwise seeking feedback on the IS/proposal and the non Composer/build-system related pieces of the MR.
exclude didnt work. Like I said, it seems to just translate to a 0 declaration under the hood.
Didnt work.
I think somewhere behind the scenes, in PHPCS, exclude translate to a regular <rule ...><severity>0 declaration.
It should certainly do this by default.
I'm wondering if it existed before, or am I just misinterpreting the docs:
If there is a baseline file in the module it will use it, but if there isn't, it will just run default values.
And do docs need an update for this MR?
its a hint for both contrib, but primarily private projects who extend coder.
Private projects may choose to force strict types, and anyone who did so before this change will suddenly find that even if they have the rule declared, Coders' declaration overrides theirs. So projects and contrib need to make their declaration more specific, in order to override Coders.
Its more a PSA than anything. But, perhaps (unlikely) there was a way for coder to introduce this change without using the exclude directive.
Drive by code drop. Not working on this for now.
Created ✨ Add a Autowire trait for plugins Active , as an interim DX measure for all existing (legacy) plugins.
FWIW each project has
https://git.drupalcode.org/project/diff/activity
And all projects master feed:
https://git.drupalcode.org/groups/project/-/activity
Click Teams tab at the top of each
Data is also available on the API
Just a note that if one uses the slevomat version also, multiple errors are reported on a line. One for each rule.
As discovered in https://github.com/previousnext/coding-standard/pull/82
Note, the changeset introduced here prevents enforcing declare(strict_types) on files.
Per
<exclude name="SlevomatCodingStandard.TypeHints.DeclareStrictTypes.DeclareStrictTypesMissing"/>
in https://git.drupalcode.org/project/coder/-/commit/eb31ae916dcb6221e8783a...
To bring back the requirement, add this line:
<rule ref="SlevomatCodingStandard.TypeHints.DeclareStrictTypes.DeclareStrictTypesMissing">
<severity>5</severity>
</rule>
Why a custom sniff over SlevomatCodingStandard.Functions.RequireTrailingCommaInDeclaration ?
Merged.
Pushed related findings to #3425971-2: Explore the posibility of a different setup for the project, composer expansion and symlinking process →
Core systems, for example LibraryDiscoveryParser, require libraries to be placed in the Drupal root or module-relative.
I have a project → which dynamically determines the location of library files, for this example it is a test module in a subdirectory of the project.
The gitlab templates as they are right now keep the module files outside the Drupal install. Then symlink the module files to a typical location inside modules/custom via symlink_project
When PHP attempts to gets the location of the file, via reflection, it is outputting the true location of the file. That is, outside of the Drupal installation.
When I try to use code in CI, it will not work since the files lie outside of the Drupal install.
My guess is that there could be other Drupal projects failing, however unless assertions are very thorough, they won't notice assets are not loading.
--
My only suggestion at this time, is to actually copy, not symlink, files to the appropriate directory.
This could be done by further modifications to the project composer.json file, by adding a path repository, and modifying symlink_project.php to instead copy files.
Or we could just set up CI so it checks out the module code where it belongs in the first place, and build a running Drupal project around it (a few directories above). Im sure there are more ways to do this...
The code I used to workaround the problem can be found at https://git.drupalcode.org/project/pinto/-/commit/ac26e665ce5f703faffbda.... Note the changes to `.gitlab-ci.yml` and a creation of a modified symlink_project, at `.gitlab/custom_symlink_project.php` (which actually copies, not symlinks)
his might lead to confusion among users who might assume these configurations are mandatory for the module's functionality.
Well, there isnt a cost (in any metric) to filling these things out. To be honest I think we just need to document it clearly, per 📌 Add guidance to readme for how to find all three configs in GTM Active .
This way we do not need to work around different script configurations. After all, the guiding principle of this module is to be as simple as possible.
Also, filling these fields implicitly educates the developer that the environments can vary easily, making their CI/Testing/Nonprod setup clear.
In my opinion, this is a wont fix.
Thanks @gapple, unfortunately I've been busy on other things so I havn't read above, and I'm good to delay work for now, as I'm using the original code happily on a project.
Fixed, just update the library.
This should do it https://github.com/dpi/pinto/pull/12
Related work @ ✨ Allow CSP to be added by render elements Needs work
This code set requires 10.2, which will be the lowest supported version of Drupal in one month. People using unsupported versions of Drupal can continue to use existing CSP releases.
Code utilises these changes:
-
https://www.drupal.org/node/3357408 →
-
https://www.drupal.org/node/3349470 →
-
https://www.drupal.org/node/3395436 →
Note: lint for 80 chars is incorrect, 120 already passed ✨ Drupal.Arrays.Array.LongLineDeclaration make me write less readable code Fixed . CI has an older coder I guess.
Some of what is discussed here is outlined in ✨ Allow CSP to be added by render elements Needs work
Merging, while lodging my protest, in order to get a 10.3 build working.
I'd prefer to fully revert this in the future.
+ support
Discussion for core: 🌱 Call PHP native functions fully qualified (like \array_key_exists()) Active
External discussion in derivative standard: https://github.com/previousnext/coding-standard/pull/69
Will be some BC breaks if extending, what do you think?
!MR41: 3.x branch
!MR42: 5.x branch
per mr
@catch I'm wondering if theres a way to just call destruct on the original.
Thinking of Symfony concepts when utilising decorators.
If a service has `calls` applied to it, it doesnt matter if it decorated or not, the calls are always applied to the original/inner service, not the outermost service.
E.g, given:
services:
dpi.test:
class: Drupal\dpi\TestService
calls:
- [ foo, ['foo!']]
dpi.deco:
class: Drupal\dpi\Deco
decorates: dpi.test
arguments:
- '@dpi.deco.inner'
calls:
- [ bar, ['bar!']]
At no point will an attempt to call dpi.deco::foo() happen.
Well, that page is less a coding standard, just a wiki page. It was added by Crell in a 2012 revision, without any references to discussions.
The standard is the living Coder project.
To me, these two parts contradict eachother:
- API documentation (in .api.php files) should use full class names.
- Note that if a class is used more than once in multiple hook signatures, it must still be "use"ed, and then only the short names of the class should be used in the function.
Is the second part referring to outside the .api.php file?
I should have created an issue here, 🐛 Container compile crash when a service decorates a destructable service Needs work