Skip to content

fix: use mixins for autocompletion #30

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

Merged
merged 2 commits into from
Jul 17, 2023
Merged

Conversation

ju5t
Copy link
Contributor

@ju5t ju5t commented Jul 14, 2023

By using mixins, an IDE will know which methods are available in the facade.

By using mixins, an IDE will know which methods are available in
the facade.
@kevindierkx
Copy link
Owner

@ju5t Thanks for the contribution!

I'm fine with the change, could you run the PHPCS fixer by running composer run phpcs-fix? I'll make sure this is included in the next release when the CI/CD stops complaining.

@ju5t
Copy link
Contributor Author

ju5t commented Jul 14, 2023

@kevindierkx no problem.

This did expose an issue with Directives.php#L101 though. PHPStan returns:

  101    Parameter #1 $host of static method Pdp\DomainNameResolver::resolve() expects Pdp\Host,
         int|Pdp\DomainNameProvider|Pdp\Host|string|Stringable|null given.

You're allowing much more in the doc block and I'm pretty sure it doesn't like that.

@kevindierkx
Copy link
Owner

@ju5t Thanks, those phpstan issues are a known issue #29 shouldn't break any code though ;)

@kevindierkx kevindierkx merged commit 8f7cc70 into kevindierkx:master Jul 17, 2023
@kevindierkx
Copy link
Owner

@ju5t Tagged 1.2.2 with your changes

@ju5t ju5t deleted the add-mixins branch July 17, 2023 12:48
@ju5t
Copy link
Contributor Author

ju5t commented Jul 17, 2023

Thanks!

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.

2 participants