Skip to content

Conversation

@CarlSchwan
Copy link
Member

@CarlSchwan CarlSchwan commented Jan 6, 2022

The issue was that we were using psalm/phar instead of vimeo/psalm. This
caused issue with the custom psalm plugin in build/psalm not using the same
php class anymore since HumbugBox was used as prefixed for the class inside
the phar.

This is using the opportunity to also update the psalm version from 3.8
to 3.18 and the php-cs-fixer too. This also consequentially updates the baseline.
There is, unfortunately, some recent regression for psalm that most of them seem
to be caused by recent commits and also a few false positives from psalm.

Signed-off-by: Carl Schwan [email protected]

The issue was that we were using psalm/phar instead of vimeo/psalm. This
caused issue with the custom psalm plugin in buildd/psalm.

This is using the opportunity to also update the psalm version from 3.8
to 3.17 and the php-cs-fixer too.

Signed-off-by: Carl Schwan <[email protected]>
@skjnldsv skjnldsv added 2. developing Work in progress bug labels Jan 6, 2022
@CarlSchwan CarlSchwan marked this pull request as draft January 6, 2022 16:04
@CarlSchwan
Copy link
Member Author

CarlSchwan commented Jan 6, 2022

:(

./vendor-bin/psalm/vendor/vimeo/psalm/psalm --update-baseline Works perfectly

composer psalm:update-baseline generate the broken baseline and doesn't report errors

I looked and both are using the same version and composer psalm:update-baseline should just call the binary inside lib/composer/bin that calls ./vendor-bin/psalm/vendor/vimeo/psalm/psalm internally. But it seems there is something broken in the way this is done...

Fixed

@CarlSchwan CarlSchwan force-pushed the fix/psaml-bin branch 3 times, most recently from f7575e9 to 47a5ea1 Compare January 6, 2022 16:52
@CarlSchwan CarlSchwan force-pushed the fix/psaml-bin branch 2 times, most recently from 3fe613d to 5022bb5 Compare January 12, 2022 15:24
@CarlSchwan CarlSchwan added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jan 12, 2022
@CarlSchwan CarlSchwan marked this pull request as ready for review January 12, 2022 15:25
@CarlSchwan
Copy link
Member Author

Works now locally :) Let's hope the ci is also happy

@CarlSchwan
Copy link
Member Author

The fact that Code scanning results / Psalm is failing is expected since it compares the baselines from the master and not with the baseline stored in the repo. There are due to the update to psalm 3.18 some new issues and also some issues that disappeared. There is nothing I can do about this, but once merged this failing CI should fix itself.

@MichaIng MichaIng added this to the Nextcloud 24 milestone Jan 12, 2022
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Good apart from the one return

Signed-off-by: Carl Schwan <[email protected]>
@CarlSchwan CarlSchwan merged commit 89d109a into master Jan 13, 2022
@CarlSchwan CarlSchwan deleted the fix/psaml-bin branch January 13, 2022 08:51
@skjnldsv
Copy link
Member

/backport 47c9c34 to stable22

@skjnldsv
Copy link
Member

/backport 47c9c34 to stable23

@backportbot-nextcloud
Copy link

The backport to stable22 failed. Please do this backport manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants