Skip to content

Use own psalm instead of a global one - #23401

Merged
MorrisJobke merged 2 commits into
masterfrom
techdebt/psalm-no-global
Oct 13, 2020
Merged

Use own psalm instead of a global one#23401
MorrisJobke merged 2 commits into
masterfrom
techdebt/psalm-no-global

Conversation

@ChristophWurst

@ChristophWurstChristophWurst commented Oct 13, 2020•
edited by MorrisJobke
Loading

Copy link
Copy Markdown
Member

Our CI used a global psalm and not one we install via composer. Hence this is waiting for random CI failure when the one in the docker image updates. We should have control over the version and use Dependabot for updates to make the manageable.

@ChristophWurstChristophWurst added 3. to review Waiting for reviews technical debt 🧱 🤔🚀 labels Oct 13, 2020
@ChristophWurstChristophWurst added this to the Nextcloud 21 milestone Oct 13, 2020
@ChristophWurstChristophWurst self-assigned this Oct 13, 2020
@ChristophWurst
ChristophWurstforce-pushed the techdebt/psalm-no-global branch from 95a4445 to 5d3808cCompareOctober 13, 2020 07:38
kesselb
kesselb approved these changes Oct 13, 2020
View reviewed changes
Comment threadcomposer.json Outdated
"require-dev": {
"nextcloud/coding-standard": "^0.3.0",
"psalm/phar": "^3.12"
"vimeo/psalm": "^3.17"

@kesselbkesselbOct 13, 2020•
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

An advantage of the phar version is less troubles with dependencies (in lib/composer)

@ChristophWurstChristophWurstOct 13, 2020

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

It is indeed. But so far there are no conflicts so I'd use the "real" one as long as possible 😉

@kesselbkesselbOct 13, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good ;) We install a lot of dependencies (like symfony/console) which are already part of 3rdparty. But lib/composer is ignored so this should work.

I would suggest to migrate the composer dev-dependencies to phive (https://phar.io/) at some point. That's more convenient to setup.

@ChristophWurst
ChristophWurstforce-pushed the techdebt/psalm-no-global branch from 5d3808c to bcd8a91CompareOctober 13, 2020 08:37
@MorrisJobkeMorrisJobke mentioned this pull request Oct 13, 2020
Closed
@MorrisJobke

MorrisJobke commented Oct 13, 2020

Copy link
Copy Markdown
Member

Both are merged ✅

@ChristophWurst
Use own psalm instead of a global one
081e9ac
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@ChristophWurst
ChristophWurstforce-pushed the techdebt/psalm-no-global branch from bcd8a91 to 081e9acCompareOctober 13, 2020 15:55
@faily-bot

faily-botBot commented Oct 13, 2020

Copy link
Copy Markdown

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 34094: failure

mysql8.0-php7.4

MorrisJobke
MorrisJobke reviewed Oct 13, 2020
View reviewed changes
Comment thread.github/workflows/static-code-analysis.yml Outdated
@MorrisJobke
Do not fail on changes to baseline.xml
106c8d7
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke
MorrisJobke merged commit 78a0449 into masterOct 13, 2020
@MorrisJobke
MorrisJobke deleted the techdebt/psalm-no-global branch October 13, 2020 21:34
@MorrisJobke

MorrisJobke commented Oct 15, 2020

Copy link
Copy Markdown
Member

Backport to stable20? It started failing in https://github.com/nextcloud/server/pull/23466/checks?check_run_id=1258325103 😢

@MorrisJobke

MorrisJobke commented Oct 15, 2020

Copy link
Copy Markdown
Member

I'm also fine with adding more stuff to the baseline in stable20, but it should not fail.

@MorrisJobkeMorrisJobke mentioned this pull request Oct 15, 2020
Merged
@MorrisJobke

MorrisJobke commented Oct 15, 2020

Copy link
Copy Markdown
Member

/backport to stable20

@MorrisJobkeMorrisJobke mentioned this pull request Oct 15, 2020
Merged
@ChristophWurst

ChristophWurst commented Oct 15, 2020

Copy link
Copy Markdown
MemberAuthor

/backport to stable20

@backportbot-nextcloudbackportbot-nextcloudBot mentioned this pull request Oct 15, 2020
Merged
@MorrisJobkeMorrisJobke mentioned this pull request Oct 29, 2020
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kesselbkesselbkesselb approved these changes
@MorrisJobkeMorrisJobkeMorrisJobke approved these changes
@nickvergessennickvergessenAwaiting requested review from nickvergessen

Assignees

@ChristophWurstChristophWurst

Labels

3. to reviewWaiting for reviewstechnical debt🧱 🤔🚀

Projects

None yet

Milestone

Nextcloud 21

Development

Successfully merging this pull request may close these issues.

3 participants

@ChristophWurst@MorrisJobke@kesselb