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

Internal: GHA "Fix Code Style" fails #142

Closed
mfn opened this issue May 19, 2022 · 7 comments
Closed

Internal: GHA "Fix Code Style" fails #142

mfn opened this issue May 19, 2022 · 7 comments
Labels
bug Something isn't working

Comments

@mfn
Copy link
Contributor

mfn commented May 19, 2022

Subject of the issue

After #141 was merged, on master https://github.com/PHP-Open-Source-Saver/jwt-auth/runs/6501560509?check_suite_focus=true was triggered and failed:

remote: error: GH006: Protected branch update failed for refs/heads/main.        
remote: error: At least 1 approving review is required by reviewers with write access. 
@mfn mfn added the bug Something isn't working label May 19, 2022
@mfn mfn self-assigned this May 19, 2022
@eschricker
Copy link
Contributor

Am I correct that this is caused by the branch protection of main? Wouldn't it be better to do the code style fixing directly on the pull request?

@mfn
Copy link
Contributor Author

mfn commented Jun 1, 2022

Wouldn't it be better to do the code style fixing directly on the pull request?

Yes! I'm pretty sure I advocated for it but no idea what/when it happened anymore 😅

It would be possible to do on master with a token with appropriate permission.

Not sure if on PRs there can be other issues like PR creators not allowing push etc., would need to test this.

PS: unassigning me, seems I'm optimistic in doing stuff but not finding time currently.

@mfn mfn removed their assignment Jun 1, 2022
@Messhias
Copy link
Collaborator

Messhias commented Jun 1, 2022

Am I correct that this is caused by the branch protection of the main? Wouldn't it be better to do the code style fixing directly on the pull request?

Yes, we can add this on workflow on PR closed/merged or when it opens since we don't need to wait until it goes to the main.

If I not wrong, there's are phpcs or something like that in the official PHP actions repository.

@eschricker
Copy link
Contributor

@Messhias do you have time to check this out and change it?

@Messhias
Copy link
Collaborator

Messhias commented Jun 1, 2022

@Messhias do you have time to check this out and change it?

I don't have time right now since I'm doing the same configurations in my current company for our PHP project.

But I'll share my php.yml that's in the workflow where I believe that we can insert into this repository workflow:

name: PHP

on:
    pull_request:
        branches:
            - master
            - dev
            - 'features/**'
            - 'hotfix/**'
            - 'release/**'
            - 'fix/**'
    push:
        branches:
            - 'features/**'
            - 'fix/**'

    schedule:
        # Run Monday morning at 3 o'clock
        #       ┌───────────── minute (0 - 59)
        #       │ ┌───────────── hour (0 - 23)
        #       │ │ ┌───────────── day of the month (1 - 31)
        #       │ │ │ ┌───────────── month (1 - 12)
        #       │ │ │ │ ┌───────────── day of the week (0 - 6)
        #       │ │ │ │ │
        #       │ │ │ │ │
        #       │ │ │ │ │
        -   cron: 0 3 * * 1

jobs:
    phpstan-ci:
        runs-on: ubuntu-latest

        steps:
            -   uses: actions/checkout@v3

            -   name: Setup PHP
                uses: shivammathur/setup-php@v2
                with:
                    php-version: 8.1
                    extensions: dom, mbstring, zip
                    coverage: none
                    tools: cs2pr

            -   name: Get composer cache directory
                id: composercache
                run: echo "::set-output name=dir::$(composer config cache-files-dir)"

            -   name: Cache dependencies
                uses: actions/cache@v2
                with:
                    path: ${{ steps.composercache.outputs.dir }}
                    key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.json') }}
                    restore-keys: ${{ runner.os }}-composer-

            -   name: Installing Composer
                run: |
                    cp .env.ci .env
                    composer install --no-ansi --no-interaction --prefer-dist --no-progress

            -   name: PHPStan
                run: composer run phpstan:test -- --error-format=checkstyle | cs2pr

            -   name: PHP-CS-Fixer
                uses: docker://oskarstark/php-cs-fixer-ga

Of course,e it's not the whole file, I'm mentioning this specific line:

-   name: PHP-CS-Fixer
     uses: docker://oskarstark/php-cs-fixer-ga

@mfn
Copy link
Contributor Author

mfn commented Feb 21, 2024

I propose we simply don't run it on main, arguments are laid out in #234

@mfn
Copy link
Contributor Author

mfn commented Feb 21, 2024

fixed with #234 , yay 🎉

@mfn mfn closed this as completed Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants