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.

Sign up for GitHub

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

feat: Allow custom phpcs coding standard #149

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

feat: Allow custom phpcs coding standard #149

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1b28742
feat: Allow custom phpcs coding standard
Tjitse-E Sep 22, 2023
ef7a0a2
Fix naming
Tjitse-E Sep 22, 2023
9911ee6
Use dealerdirect/phpcodesniffer-composer-installer to auto register c…
Tjitse-E Sep 22, 2023
478d81f
Fix work dir
Tjitse-E Sep 22, 2023
6c6aebc
Test
Tjitse-E Sep 22, 2023
272d75f
Fix
Tjitse-E Sep 22, 2023
76ca691
Add docs
Tjitse-E Sep 22, 2023
e8d20c9
Fix docs
Tjitse-E Sep 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions coding-standard/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,6 @@ jobs:
severity: 8 # Optional, will use phpcs default of 5 if not specified.
warning_severity: 4 # Optional, will use severity value if not specified.
error_severity: 7 # Optional, will use severity value if not specified.
coding_standard: "Magento2"
custom_coding_standard_repo: "your/custom-repo"
```
31 changes: 24 additions & 7 deletions coding-standard/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,17 @@ inputs:
version:
required: false
description: "The version of the coding standard to use. If not provided, will use the latest version."

severity:
required: false
default: ""
description: "The minimum severity required to display an error or warning (default: 5)"

warning_severity:
required: false
default: ""
description: "The minimum severity required to display a warning"

error_severity:
required: false
default: ""
Expand All @@ -42,6 +42,16 @@ inputs:
default: 'false'
required: false

coding_standard:
description: 'The coding standard to use, defaults to Magento2'
default: 'Magento2'
required: false

custom_coding_standard_repo:
description: 'The repository to use for the coding standard, used if you want to use a custom coding standard'
default: ''
required: false

runs:
using: composite
steps:
Expand Down Expand Up @@ -79,15 +89,21 @@ runs:
run: composer config allow-plugins.dealerdirect/phpcodesniffer-composer-installer true --global
if: steps.is-allow-plugins-available.outputs.result < 1

- name: Install Coding Standard
- name: Install composer require dealerdirect/phpcodesniffer-composer-installer
shell: bash
working-directory: standard
run: composer require dealerdirect/phpcodesniffer-composer-installer

- name: Install Magento2 Coding Standard
Copy link
Member

@damienwebdev damienwebdev Sep 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Are these two things (the custom standard install and the Magento standard install) mutually exclusive? Wouldn't custom_standard have a dep on Magento?

shell: bash
working-directory: standard
run: composer require "magento/magento-coding-standard:${{ inputs.version || '*' }}"

- name: Register Coding Standard
- name: Install Custom Coding Standard
Copy link
Member

@damienwebdev damienwebdev Sep 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I foresee two situations for this action to be used:

  1. Extensions
  2. Stores

both of them will often have this extra repo already included in require-dev. Should we be the ones to install this? It seems like an unnecessary network IO if it is already installed. Perhaps #133 applies to this as well?

I don't personally think custom_coding_standard_repo is a good idea, though I like the ability to change the standard via coding_standard

if: ${{ inputs.custom_coding_standard_repo }}
shell: bash
working-directory: standard
run: vendor/bin/phpcs --config-set installed_paths ${{ github.workspace }}/standard/vendor/magento/magento-coding-standard,${{ github.workspace }}/standard/vendor/phpcompatibility/php-compatibility
run: composer require "${{ inputs.custom_coding_standard_repo }}"

- name: Set ignore warnings flag
shell: bash
Expand All @@ -105,7 +121,8 @@ runs:
- name: Coding Standard Check
shell: bash
run: |
../standard/vendor/bin/phpcs --standard=Magento2 \
../standard/vendor/bin/phpcs \
--standard="${{ inputs.coding_standard }}" \
$([ -n "${{ inputs.severity }}" ] && echo "--severity=${{ inputs.severity }}") \
$([ -n "${{ inputs.warning_severity }}" ] && echo "--warning-severity=${{ inputs.warning_severity }}") \
$([ -n "${{ inputs.error_severity }}" ] && echo "--error-severity=${{ inputs.error_severity }}") \
Expand Down