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

Don't attempt to read baseline before it was generated #5961

Merged
merged 1 commit into from
Jun 21, 2021

Conversation

weirdan
Copy link
Collaborator

@weirdan weirdan commented Jun 20, 2021

Fixes #5960

Previous fix would not return generated baseline.

@weirdan weirdan marked this pull request as draft June 20, 2021 23:16
@muglug
Copy link
Collaborator

muglug commented Jun 20, 2021

Sorry, mind running through what steps weren't happening before that are now?

@weirdan
Copy link
Collaborator Author

weirdan commented Jun 20, 2021

4.7.3 would write baseline, read it back and then use in IssueBuffer::finish(). I'm checking whether that was actually required.

@weirdan
Copy link
Collaborator Author

weirdan commented Jun 20, 2021

@muglug this prevents a cosmetic issue where issues added to the baseline during current run are still reported:

image

@weirdan weirdan marked this pull request as ready for review June 20, 2021 23:40
@muglug muglug merged commit 52d2692 into vimeo:master Jun 21, 2021
@muglug
Copy link
Collaborator

muglug commented Jun 21, 2021

Thanks! Not sufficient for a separate release, but this is great.

@orklah
Copy link
Collaborator

orklah commented Jul 5, 2021

@weirdan This cosmetic issue is exactly what I was trying to achieve in #5838 after discussing it in #5831 :)

When running with a baseline, it become impossible to know what issues have been added

On our project, Psalm takes too long to run to hope adding it in CI, so we have a setup that runs Psalm regularly and saves the errors in baseline. Each time we see new errors, we handle them and report to whoever may be concerned.

Without this, we need to run Psalm once without baseline, takes the issues and send them, then re-run Psalm with the baseline.

Can we revert this change or introduce a new option to return to the described behaviour?

@weirdan
Copy link
Collaborator Author

weirdan commented Jul 5, 2021

On our project, Psalm takes too long to run to hope adding it in CI

Even with cache enabled?

@weirdan
Copy link
Collaborator Author

weirdan commented Jul 5, 2021

@orklah and I thought I botched that during refactoring, because I was comparing with last good release (4.7.3) 😆

Obligatory xkcd:1172.

In that workflow of yours, do you output machine-readable data (e.g. --output-format/--report)?

@orklah
Copy link
Collaborator

orklah commented Jul 6, 2021

Even with cache enabled?

Trying to run Psalm without --no-cache brings the whole server down in about 15-20 minutes (a run without cache takes 10 minutes). I never could go to the bottom of this issue (I still hope I will one day !)

Obligatory xkcd:1172.

😄

In that workflow of yours, do you output machine-readable data (e.g. --output-format/--report)?

I don't remember the option we use, but we also retrieve in a file the whole list of issues reported by Psalm but the baseline is ignored when doing that so we retrieve every issue

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.

Unable to create baseline in psalm 4.8.0
3 participants