Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a generator for single directory components #109

Merged
merged 18 commits into from
Aug 1, 2023

Conversation

PierrePaul
Copy link
Contributor

Resolves issue https://www.drupal.org/project/cl_generator/issues/3365150
Add a generator for Single Directory Component with a first basic functional test.

src/Command/SingleDirectoryComponent.php Outdated Show resolved Hide resolved
src/Command/SingleDirectoryComponent.php Outdated Show resolved Hide resolved
@PierrePaul
Copy link
Contributor Author

I think I have fixed all the issues, except 3 from linting :

ERROR: ArgumentTypeCoercion - src/Command/SingleDirectoryComponent.php:50:7 - Argument 1 of DrupalCodeGenerator\Command\SingleDirectoryComponent::__construct expects Drupal\Core\Extension\ModuleHandlerInterface, but parent type object provided (see https://psalm.dev/193)
      $container->get('module_handler'),

I'll need some guidance on that :)

@Chi-teck Chi-teck changed the title 3365150 drush 12 rewrite Provide a generator for single directory components Jul 28, 2023
@Chi-teck
Copy link
Owner

psalm needs to be informed about type of services that Drupal container returns.
That can be done through phpstorm.meta.php file.

src/Command/SingleDirectoryComponent.php Outdated Show resolved Hide resolved
src/Command/SingleDirectoryComponent.php Show resolved Hide resolved
src/Command/SingleDirectoryComponent.php Outdated Show resolved Hide resolved
src/Command/SingleDirectoryComponent.php Outdated Show resolved Hide resolved
@Chi-teck
Copy link
Owner

UnusedPsalmSuppress

@PierrePaul This has been resolved in 3.x branch. You might need merge it into your branch.

src/Validator/RegExp.php Outdated Show resolved Hide resolved
@Chi-teck
Copy link
Owner

RedundantPropertyInitializationCheck

symfony/dependency-injection has improved the container documentation in v6.3.2 so that the suppression has become redundant. The problem here is that we cannot apply the suppression conditionally depending on symfony version. Hence, I've to update the package.

UnusedPsalmSuppress

Psalm typically adds new violations with each release. The latest one 5.14.0 has been released just yesterday which brought some mess to this PR. This has been fixed in 3.x branch. Also I decided to pin the psalm version to make builds more predictable.

I've just merged 3.x branch into your fork. It should resolve the psalm issues.

@PierrePaul
Copy link
Contributor Author

Awesome thank you! So all done?

@Chi-teck
Copy link
Owner

Chi-teck commented Aug 1, 2023

Yes. I'll likely apply some minor changes to the code later to make it consistent with other generators. Also will check the generated SDC itself before the next release.

Thank you for the hard work!

@Chi-teck Chi-teck merged commit 87fe988 into Chi-teck:3.x Aug 1, 2023
8 checks passed
@PierrePaul PierrePaul deleted the 3365150-drush-12-rewrite branch August 2, 2023 12:46
@Chi-teck
Copy link
Owner

Chi-teck commented Aug 6, 2023

I've removed the "status" question. The looks unlikely that someone will generate obsolete or deprecated components. And this is not a big deal to change this property in YML file after generation. Also replaced examples question with todo statement in the YML definition.
The test has been extended to check assets and props.

The generator is included to DCG 3.2.0 release.

@PierrePaul
Copy link
Contributor Author

Looking good! Do you want me to open up a MR to update DCG on the drush repo?

@Chi-teck
Copy link
Owner

Chi-teck commented Aug 7, 2023

That is not necessary. Composer installs the most recent stable release by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants