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

Support non annotated property type #456

Merged
merged 4 commits into from
Jan 29, 2021
Merged

Conversation

sasezaki
Copy link
Contributor

fix #455

@smoench smoench added this to the next milestone Jan 25, 2021
@smoench
Copy link
Contributor

smoench commented Jan 25, 2021

@sasezaki Good catch. LGTM

Is there anything left or need help with something?

@sasezaki
Copy link
Contributor Author

@smoench
thanks for review.

Is there anything left or need help with something?

Just , it left Coding Standards CI Failed issue.

Files that were not fixed due to errors reported during linting before fixing:

  1. /home/runner/work/deptrac/deptrac/tests/AstRunner/Resolver/fixtures/PropertyTypeDependency.php
    https://github.com/sensiolabs-de/deptrac/pull/456/checks?check_run_id=1753259847

Can I add this PropertyTypeDependency.php into .php_cs.dist exclude ?

@sasezaki
Copy link
Contributor Author

Can I add this PropertyTypeDependency.php into .php_cs.dist exclude ?

I did try with notPath.

$finder = PhpCsFixer\Finder::create()
    ->in([__DIR__.'/config', __DIR__.'/src', __DIR__.'/tests'])
    ->exclude('Fixtures')
    ->notPath('tests/AstRunner/Resolver/fixtures/PropertyTypeDependency.php')
    ->append([__DIR__.'/deptrac.php']);

but Finder still does not exclude PropertyTypeDependency.php file... 😢

@smoench
Copy link
Contributor

smoench commented Jan 29, 2021

Can you try to place your fixture file in the directory Fixtures (with a capital F). With the existing ->exclude('Fixtures') in .php_cs.dist your fixture file should be excluded.

@sasezaki
Copy link
Contributor Author

@smoench
Shall I rename tests/AstRunner/Resolver/fixtures to tests/AstRunner/Resolver/Fixtures ?
or put PropertyTypeDependency.php into new Fixtures directory ?

@smoench
Copy link
Contributor

smoench commented Jan 29, 2021

That would be awesome but it's up to you. The rename would cause other test case adjustments. Lets try put PropertyTypeDependency.php into new Fixtures directory first if it's working you can try the rename.

@sasezaki
Copy link
Contributor Author

@smoench
thank you for advice.
I did rename fixtures to Fixtures.

@sasezaki sasezaki marked this pull request as ready for review January 29, 2021 10:22
@smoench smoench merged commit b996327 into qossmic:master Jan 29, 2021
@smoench
Copy link
Contributor

smoench commented Jan 29, 2021

Thank you @sasezaki! Good job!

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

Successfully merging this pull request may close these issues.

can not find declaration property type dependency.
2 participants