Skip to content

Conversation

@orklah
Copy link
Contributor

@orklah orklah commented Feb 9, 2020

Also fixed ignored errors (FYI, PHPStan ignore rule are regexes. The previous error had "|string>" at the end. The effect was that every error ending by "|string>" was being ignored. Newer versions of PHPStan now warn about that)

@orklah
Copy link
Contributor Author

orklah commented Feb 9, 2020

I had to double down on a hack in the code documentation: https://github.com/phpDocumentor/ReflectionDocBlock/pull/201/files#diff-24bbef7e3801a14d93d080b7875d5ef6R179

This is due to a bug in PHPStan's extension for Webmozart/Assert. We'll be able to remove it when phpstan/phpstan-webmozart-assert#18 is fixed.

@orklah
Copy link
Contributor Author

orklah commented Feb 9, 2020

For consistency with the main project, I replaced some @psalm by @phpstan annotations. This shouldn't have any impact.

@orklah
Copy link
Contributor Author

orklah commented Feb 9, 2020

@jaapio Weird, CI seems broken on pull requests. It only ran Scrutinizer...

@jaapio
Copy link
Member

jaapio commented Feb 10, 2020

Build on pr is fixed. Please rebase this PR, let me know when it is ready to merge 👍

@orklah
Copy link
Contributor Author

orklah commented Feb 10, 2020

This is ready for review

Copy link
Member

@jaapio jaapio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for these changes.
Another step forward to a better typed lib!

I delay the actual merge because I want to add some additional tooling to the CI to ensure we are not breaking backwards compatiblity. That needs some time. I don't expect this pr to change anything. But since I'm not familiar with the specific tool I don't want to have any commits in here right now that possibly have influence on the tool.

@orklah
Copy link
Contributor Author

orklah commented Feb 10, 2020

No worries, I know this is highly sensitive. I don't want to break anything that will annoy a lot of people :D

@jaapio jaapio merged commit b21eaf3 into phpDocumentor:master Feb 12, 2020
@jaapio
Copy link
Member

jaapio commented Feb 12, 2020

No Bc breaks on api level, So I expect this to be safe :-)

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