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 SYSTOPIA extension template with QA tools and GitHub actions #733

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jensschuppe
Copy link
Collaborator

phpcs and phpstan GitHub actions are set to only run on workflow_dispatch due to too many errors.

@jensschuppe jensschuppe added enhancement status:needs review Code needs review and testing labels Aug 19, 2024
phpcs and phpstan GitHub actions are set to only run on workflow_dispatch due to too many errors.
@dontub
Copy link
Collaborator

dontub commented Sep 2, 2024

I'm afraid if the actions only run workflow_dispatch the rules won't be regarded for new code. Have you considered to limit phpcs and phpstan to the Civi directory?

@jensschuppe
Copy link
Collaborator Author

Actually, the point was to be able to already include the desired rules but not run them on GitHub for now, until the code can be cleaned up. Restricting to only small parts of code will disable all checks locally as well (which I don't find useful tbh).

workflow_dispatch allows you to manually run things on GitHub when desired. But you are sure, this requires every contributor or reviewer to run the checks locally for QA. I'm not sure what's the better approach …

@dontub
Copy link
Collaborator

dontub commented Sep 2, 2024

I agree, restricting the checks to only a smaller part is definitely not ideal. Though I'm afraid if one runs composer phpcs or composer phpstan and gets a bunch of errors it might not be really helpful and will be ignored.

It is always possible to add paths in a local configuration file, if required...

@jensschuppe
Copy link
Collaborator Author

Sure, feel free to try that in the phpstan.neon.template file. I'm not married to either approach 😏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement status:needs review Code needs review and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants