-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Updated contribution guide with "Automated Feedback" #15024
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
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nicolas-grekas
approved these changes
Feb 24, 2021
Thank you |
wouterj
reviewed
Feb 24, 2021
fabpot
reviewed
Feb 24, 2021
39f1794
to
ffc79c0
Compare
nicolas-grekas
added a commit
to symfony/symfony
that referenced
this pull request
Feb 25, 2021
This PR was squashed before being merged into the 4.4 branch. Discussion ---------- Adding a Github action to run Psalm | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | | License | MIT | Doc PR | symfony/symfony-docs#15024 I've seen sometimes that we've forgotten to add `\` before `Throwable` or that we refer to a class that does not exist. One could argue that the code is not properly tested, but somehow these PRs still get merged. (And quickly fixed in a follow up PR). I suggest to add psalm to check every PR for some errors that can be found with a static analyser. This is to help/automate the PR review process. All psalm errors found should be reviewed and discussed. The maintainers can decide to ignore some warnings if they want to. (Ie false positives) This PR is about “Psalm PR review”. It does not try to fix “Psalm compatibility”. Psalm compatibility is a separate issue that should be discussed separate from the "Psalm PR review". I currently plan to follow up with the more controversial topic of "Should we make Symfony more compatible with Psalm or not". Commits ------- c5ed24d Adding a Github action to run Psalm
The related PR is merged. |
ffc79c0
to
795dcb7
Compare
Thank you Tobias! We merged this in 4.4 and up. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adding a note about psalm.
Related PR: symfony/symfony#40291