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

Fix IDE warnings #20

Merged
merged 2 commits into from
Sep 24, 2021
Merged

Fix IDE warnings #20

merged 2 commits into from
Sep 24, 2021

Conversation

mlocati
Copy link
Contributor

@mlocati mlocati commented Sep 24, 2021

This PR fixes a few warnings raised by my IDE

@KorvinSzanto
Copy link
Member

Very nice, thank you!
Can your .shim file be moved to individual stubs like we have in /.psalm/stubs?

@mlocati
Copy link
Contributor Author

mlocati commented Sep 24, 2021

Should I rename them as .php.stub?

@KorvinSzanto
Copy link
Member

I named them .php.stub, the psalm docs use .phpstub: https://psalm.dev/docs/running_psalm/plugins/authoring_plugins/ It honestly doesn't matter to me what we use as long as we're consistent. Make sure to add them to the psalm.xml as well.

I'm not sure if what you're trying to resolve is resolved by using the stub files, if not maybe we can just have both.

@mlocati
Copy link
Contributor Author

mlocati commented Sep 24, 2021

Should I keep the php stub files in the .concrete directory? I'd prefer to have all of them in the same place (.psalm/stubs)

@KorvinSzanto
Copy link
Member

KorvinSzanto commented Sep 24, 2021

Should I keep the php stub files in the .concrete directory? I'd prefer to have all of them in the same place (.psalm/stubs)

If you mean that index.maintenance.php then yeah I want that to stay in the .concrete directory since it's more of a template than a stub.

If you mean shim.php from this PR then yeah I'd prefer those classes in individual files underneath .psalm/stubs like the other two stubs are assuming that still fixes the IDE warnings you're mentioning.

@mlocati
Copy link
Contributor Author

mlocati commented Sep 24, 2021

If you mean shim.php from this PR then yeah I'd prefer those classes in individual files underneath .psalm/stubs like the other two stubs are assuming that still fixes the IDE warnings you're mentioning.

That's what I meant, and what I did: 9e9e614

@KorvinSzanto KorvinSzanto merged commit 4483ded into concretecms:main Sep 24, 2021
@mlocati mlocati deleted the fix-ide-warnings branch September 24, 2021 17:21
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