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

Add Github workflow for checking PHP quality #2

Merged
merged 9 commits into from
Sep 21, 2022

Conversation

petitphp
Copy link
Member

No description provided.

@petitphp petitphp force-pushed the feature/github-workflows branch from c309e1d to a326065 Compare September 17, 2022 09:07
@petitphp petitphp marked this pull request as ready for review September 17, 2022 09:42
checks:
name: Lint JS
runs-on: ubuntu-latest
if: ${{ github.repository == 'BeAPI/multisite-shared-blocks' || github.event_name == 'pull_request' }}
Copy link
Member

Choose a reason for hiding this comment

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

Pourquoi tester le nom du repo ? ou l'event name ?
La condition au dessus devrait suffire

Copy link
Member Author

Choose a reason for hiding this comment

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

C'est repris de ce que fait WordPress ou Yoast dans ses workflows. Je crois que c'est une sécurité en cas ou une personne fork un projet.

Copy link
Member

Choose a reason for hiding this comment

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

Ah ok, inutile à notre échelle je pense.

Copy link
Member Author

Choose a reason for hiding this comment

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

Pour référence : WordPress/gutenberg#32114

Copy link
Member Author

Choose a reason for hiding this comment

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

C'est modifié.

checks:
name: Codesniffer
runs-on: ubuntu-latest
if: ${{ github.repository == 'BeAPI/multisite-shared-blocks' || github.event_name == 'pull_request' }}
Copy link
Member

Choose a reason for hiding this comment

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

Pourquoi tester le nom du repo ? ou l'event name ?
La condition au dessus devrait suffire

Copy link
Member Author

Choose a reason for hiding this comment

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

C'est modifié.

unit-php:
name: PHP
runs-on: ubuntu-latest
if: ${{ github.repository == 'BeAPI/multisite-shared-blocks' || github.event_name == 'pull_request' }}
Copy link
Member

Choose a reason for hiding this comment

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

Pourquoi tester le nom du repo ? ou l'event name ?
La condition au dessus devrait suffire

Copy link
Member Author

Choose a reason for hiding this comment

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

C'est modifié.


- name: Run unit tests
run: npm run test:php
if: ${{ success() || failure() }}
Copy link
Member

Choose a reason for hiding this comment

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

Pas compris l'intéret de la ligne, si la commande plante alors l'action sera considérée comme fausse et donc OK.

Copy link
Member Author

Choose a reason for hiding this comment

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

C'est repris de ce que fait Gutenberg dans son workflow. Je peux tester sans.

Copy link
Member

Choose a reason for hiding this comment

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

Bah là ça dit que ça passe que se soit une erreur ou un succès, ça me parait chelou, laissons-le échouer si besoin

Copy link
Member Author

Choose a reason for hiding this comment

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

C'est modifié.

checks:
name: Lint JS
runs-on: ubuntu-latest
if: ${{ github.repository == 'BeAPI/multisite-shared-blocks' || github.event_name == 'pull_request' }}
Copy link
Member

Choose a reason for hiding this comment

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

Ah ok, inutile à notre échelle je pense.


- name: Run unit tests
run: npm run test:php
if: ${{ success() || failure() }}
Copy link
Member

Choose a reason for hiding this comment

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

Bah là ça dit que ça passe que se soit une erreur ou un succès, ça me parait chelou, laissons-le échouer si besoin

@petitphp petitphp requested a review from Rahe September 19, 2022 15:39
@petitphp petitphp force-pushed the feature/github-workflows branch from 5552526 to 3cbbd56 Compare September 20, 2022 20:51
@petitphp petitphp merged commit b73e694 into main Sep 21, 2022
@petitphp petitphp deleted the feature/github-workflows branch September 21, 2022 07:53
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.

2 participants