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

Psalm integration #15

Merged
merged 8 commits into from
Jun 26, 2022
Merged

Psalm integration #15

merged 8 commits into from
Jun 26, 2022

Conversation

jslmorrison
Copy link
Contributor

Q A
Documentation no
Bugfix no
BC Break /no
New Feature yes/
RFC no
QA yes

Description

Add vimeo/psalm integration.

@Ocramius Ocramius self-assigned this Jun 25, 2022
@Ocramius Ocramius added the Enhancement New feature or request label Jun 25, 2022
@Ocramius Ocramius added this to the 1.2.0 milestone Jun 25, 2022
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

Baseline needs expanding 🤔

psalm.xml Outdated Show resolved Hide resolved
Signed-off-by: John Morrison <[email protected]>
Also requires the phpunit plugin for Psalm as generating the baseline xml file throws an exception without it.

Signed-off-by: John Morrison <[email protected]>
Replace deprecated 'totallyTyped' attribute with 'errorLevel'.

Signed-off-by: John Morrison <[email protected]>
@Ocramius
Copy link
Member

See CI failures: the generated baseline is missing most reported errors 🤔

@jslmorrison
Copy link
Contributor Author

See CI failures: the generated baseline is missing most reported errors

Hi - I don't see these errors when running static-analysis locally. Do you want them suppressed by expanding the baseline in the following manner?

<PropertyNotSetInConstructor>
    <errorLevel type="suppress">
        <referencedProperty name="Mezzio\Migration\MigrateCommand::$input" />
    </errorLevel>
</PropertyNotSetInConstructor>

@Ocramius
Copy link
Member

No, they should just appear as "known issues" in the baseline 🤔

Check what CI did run here: possibly some differences between local setup and this?

Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

Besides the duplication, this looks much better now, thanks @jslmorrison!

psalm.xml Show resolved Hide resolved
Remove duplicate block

Signed-off-by: John Morrison <[email protected]>
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

Thanks @jslmorrison!

@Ocramius Ocramius merged commit df4c59e into mezzio:1.2.x Jun 26, 2022
@Ocramius Ocramius changed the title Feature/psalm integration Psalm integration Jun 26, 2022
@jslmorrison jslmorrison deleted the feature/psalm-integration branch June 28, 2022 13:10
@gsteel gsteel linked an issue Jul 27, 2022 that may be closed by this pull request
8 tasks
@gsteel gsteel mentioned this pull request Jul 27, 2022
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Psalm integration
2 participants