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

Bump PHP to 8.1 #43

Merged
merged 1 commit into from
Dec 29, 2022
Merged

Bump PHP to 8.1 #43

merged 1 commit into from
Dec 29, 2022

Conversation

wow-apps
Copy link
Contributor

@wow-apps wow-apps commented Dec 27, 2022

Solving issue #42

Making #39 deprecated

@skjnldsv
Copy link

@muglug @Nyholm @Taluu @simPod @orklah is this repo still maintained? :)
Thanks and have a great day! 🎉

@Nyholm
Copy link
Contributor

Nyholm commented Dec 29, 2022

It is only Matthew that has write access as far as I know. He left Vimeo. I haven't seen any other Psalm maintainers here.

FYI, A lot has happened since this action was published. You may get some inspiration from here.

  psalm:
    name: Psalm
    runs-on: ubuntu-22.04
    steps:
      - name: Checkout code
        uses: actions/checkout@v3

      - name: Setup PHP
        uses: shivammathur/setup-php@v2
        with:
          php-version: 8.1
          extensions: apcu, redis
          coverage: none

      - name: Download dependencies
        uses: ramsey/composer-install@v2

      - name: Psalm
        run: vendor/bin/psalm --no-progress --output-format=github

@orklah
Copy link
Contributor

orklah commented Dec 29, 2022

Hey :)

I do have maintainer access here. I did not merge yet because I don't actually know very much about this repo and I was waiting to see if another maintainer would respond

I'm a bit surprised that the solution to a parse error would be to upgrade php here. When Psalm analyze the code, it should do it in the correct PHP version, and out of Psalm's analysis, composer should take the lead to prevent using packages that are not compatible.

EDIT: I realize I said about the same thing in #39 (comment) . The situation did not change much since then, Psalm still support 7.4 in theory, so while I'm not against merging, I'm not sure why it's needed and I don't want to bother people if it could have a negative impact

EDIT2: just saw comments in #31 . Seems like legitimate use cases :)

@orklah orklah merged commit 6d742de into psalm:master Dec 29, 2022
@orklah
Copy link
Contributor

orklah commented Dec 29, 2022

Thanks!

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.

4 participants