matomo-org / plugin-GoogleAnalyticsImporter

Google Analytics to Matomo importer
24 stars 14 forks source link

Added test for phpcs #519

Closed AltamashShaikh closed 2 months ago

AltamashShaikh commented 6 months ago

Description:

Added test for phpcs

Review

sgiehl commented 6 months ago

@AltamashShaikh I guess it should be better to have a separate action for this. The test yml file can be auto generated with a command. Doing that would throw away the PHPCS part. Also using strict PSR12 won't easily work without introducing braking changes. This is the CS we are currently using for core: https://github.com/matomo-org/matomo/blob/5.x-dev/phpcs.xml Note: We are currently working on enabling a few more rules to get closer to PSR12... At some point we need to create a new repository that holds our PHPCS rules, and maybe even allows to add custom rules in the future. Every repo would than need to require those rules and simply run PHPCS with them.

AltamashShaikh commented 6 months ago

@AltamashShaikh I guess it should be better to have a separate action for this. The test yml file can be auto generated with a command. Doing that would throw away the PHPCS part. Also using strict PSR12 won't easily work without introducing braking changes. This is the CS we are currently using for core: https://github.com/matomo-org/matomo/blob/5.x-dev/phpcs.xml Note: We are currently working on enabling a few more rules to get closer to PSR12... At some point we need to create a new repository that holds our PHPCS rules, and maybe even allows to add custom rules in the future. Every repo would than need to require those rules and simply run PHPCS with them.

@sgiehl Yes will move to a separate action and as suggested will make the related changes

michalkleiner commented 6 months ago

I would exclude vendor and all the prefixed dependencies from the code check, and only apply the action to actual plugin code. As mentioned before in a meeting, and as Stefan also suggests, longterm we should aim at having something like matomo-org/matomo-coding-standards repo that can be shared across core and plugins and cover other areas like JSLint or prettier rules etc., not just PHP. It's a start here though. Github actions can also be reused from other repo, so here we could simply reference the core repo action (check examples in https://docs.github.com/en/actions/using-workflows/reusing-workflows, mainly https://docs.github.com/en/actions/using-workflows/reusing-workflows#calling-a-reusable-workflow).

AltamashShaikh commented 6 months ago

e reused from other repo, so here we could simply reference the core repo action (

@michalkleiner I can try your suggestions and update you accordingly :+1:

AltamashShaikh commented 2 months ago

Closing this in favor of #547