Add improved codesniffer support. #151
Conversation
* fix: support drupal 11 * fix: improve code * fix: default installation * fix: update mariadb version to 10.6 * fix: update ddev-gitpod image (latest ddev version) * fix: lazygit binaries * fix: remove legacy settings from gitpod.yml * fix: use DP_CORE_VERSION when there's no issue_fork * fix: reorganize scripts order * Update .gitpod/drupal/drupalpod-setup/ddev_setup.sh Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Allows fallback setup when DrupalPod is called with no DP_PROJECT_TYPE. That is the case when DrupalPod is called without using the browser extension.
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
codebymikey
force-pushed
the
codesniffer-support
branch
from
3f906d0 to
8b90f63
Compare
WalkthroughThis update introduces new scripts and modifications to existing ones to improve the handling of PHP_CodeSniffer tools ( Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
.gitpod/drupal/drupalpod-setup/drupalpod-setup.sh (1)
Line range hint
119-119: Correct the syntax for better script robustness.- if [ ! -f "${GITPOD_REPO_ROOT}"/.drupalpod_initiated ]; then + if [ ! -f "${GITPOD_REPO_ROOT}"/.drupalpod_initiated ] + thenShellcheck warns about using a semicolon or linefeed before 'then'. It's a good practice to place 'then' on a new line for clarity and to avoid potential script errors.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- .ddev/commands/web/phpcbf (1 hunks)
- .ddev/commands/web/phpcs (1 hunks)
- .gitpod/drupal/drupalpod-setup/drupalpod-setup.sh (1 hunks)
- .gitpod/utils/env-setup.sh (1 hunks)
- .gitpod/utils/script-templates/ddev-phpcbf.template.sh (1 hunks)
- .gitpod/utils/script-templates/ddev-phpcs.template.sh (1 hunks)
- .gitpod/utils/script-templates/ddev-phpunit.template.sh (1 hunks)
Files skipped from review due to trivial changes (6)
- .ddev/commands/web/phpcbf
- .ddev/commands/web/phpcs
- .gitpod/utils/env-setup.sh
- .gitpod/utils/script-templates/ddev-phpcbf.template.sh
- .gitpod/utils/script-templates/ddev-phpcs.template.sh
- .gitpod/utils/script-templates/ddev-phpunit.template.sh
Additional context used
Shellcheck
.gitpod/drupal/drupalpod-setup/drupalpod-setup.sh
[info] 51-51: Not following: ./fallback_setup.sh was not specified as input (see shellcheck -x).
[info] 54-54: Not following: ./git_setup.sh was not specified as input (see shellcheck -x).
[info] 63-63: Not following: ./ddev_setup.sh was not specified as input (see shellcheck -x).
[info] 68-68: Not following: ./contrib_modules_setup.sh was not specified as input (see shellcheck -x).
[info] 69-69: Not following: ./cleanup.sh was not specified as input (see shellcheck -x).
[info] 70-70: Not following: ./composer_setup.sh was not specified as input (see shellcheck -x).
[info] 79-79: Not following: ./drupal_setup_core.sh was not specified as input (see shellcheck -x).
[info] 82-82: Not following: ./drupal_setup_contrib.sh was not specified as input (see shellcheck -x).
[warning] 119-119: Use semicolon or linefeed before 'then' (or quote to make it literal).
Additional comments not posted (1)
.gitpod/drupal/drupalpod-setup/drupalpod-setup.sh (1)
89-91: Ensure PHP Codesniffer'sinstalled_pathsis correctly set.The conditional check using
ddev phpcs --config-show | grep -q 'installed_paths:'before setting theinstalled_pathsis a good practice. It prevents unnecessary reconfiguration if it's already set, which can be particularly useful in environments where the setup script might be executed multiple times.
This ensures that absolute paths on the host are automatically converted to the absolute path within the container without issue.
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- .ddev/commands/host/exec_dir (1 hunks)
- .gitpod/utils/script-templates/ddev-phpcbf.template.sh (1 hunks)
- .gitpod/utils/script-templates/ddev-phpcs.template.sh (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- .gitpod/utils/script-templates/ddev-phpcbf.template.sh
- .gitpod/utils/script-templates/ddev-phpcs.template.sh
Additional comments not posted (1)
.ddev/commands/host/exec_dir (1)
24-24: Validate the final command execution to ensure it correctly uses the modified arguments.
| for arg in "$@"; do | ||
| new_arg="${arg//${GITPOD_REPO_ROOT}//var/www/html}" | ||
| arguments+=("$new_arg") | ||
| done |
There was a problem hiding this comment.
Tip
Codebase Verification
The path replacement logic in the loop does not handle edge cases where the substring appears multiple times in the path. This can lead to incorrect replacements, as demonstrated by the script output.
- The replacement logic should be adjusted to ensure only the intended part of the path is replaced.
Analysis chain
Check the path replacement logic for potential edge cases.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the path replacement logic handles all edge cases.
# Test: Simulate a scenario where the path might not be replaced correctly.
# Example: When the path contains similar substrings or nested paths.
echo "/path/to/repo/path/to/repo/file" | sed "s|/path/to/repo|/new/path|g"
Length of output: 98
| @@ -9,4 +9,16 @@ if [ -z "$1" ]; then | |||
| exit 1 | |||
| fi | |||
|
|
|||
| ddev exec -d /var/www/html/"$(echo "$PWD" | cut -d'/' -f4-)" "$@" | |||
| if [ "${GITPOD_NORMALIZE_EXEC_ARGUMENTS:-1}" = 1 ] && [ -n "${GITPOD_REPO_ROOT}" ]; then | |||
There was a problem hiding this comment.
Tip
Codebase Verification
The environment variable GITPOD_REPO_ROOT is set in the .gitpod.yml file, but there is no evidence of GITPOD_NORMALIZE_EXEC_ARGUMENTS being set anywhere in the codebase or configuration files. This could lead to the condition in the script not being met.
- Ensure that
GITPOD_NORMALIZE_EXEC_ARGUMENTSis set appropriately in the environment or configuration files.
Analysis chain
Ensure GITPOD_NORMALIZE_EXEC_ARGUMENTS is properly set in the environment.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the environment variable `GITPOD_NORMALIZE_EXEC_ARGUMENTS` is set.
# Test: Check the environment variable. Expect: It should be set to 1.
echo $GITPOD_NORMALIZE_EXEC_ARGUMENTS
Length of output: 7
Script:
#!/bin/bash
# Description: Verify the environment variable `GITPOD_REPO_ROOT` is set.
# Test: Check the environment variable. Expect: It should be non-empty.
echo $GITPOD_REPO_ROOT
Length of output: 7
Script:
#!/bin/bash
# Description: Search for the setting of `GITPOD_NORMALIZE_EXEC_ARGUMENTS` and `GITPOD_REPO_ROOT` in the codebase.
# Search for `GITPOD_NORMALIZE_EXEC_ARGUMENTS`
rg 'GITPOD_NORMALIZE_EXEC_ARGUMENTS'
# Search for `GITPOD_REPO_ROOT`
rg 'GITPOD_REPO_ROOT'
Length of output: 58
Script:
#!/bin/bash
# Description: Search for the setting of `GITPOD_NORMALIZE_EXEC_ARGUMENTS` and `GITPOD_REPO_ROOT` in configuration files.
# Search for `GITPOD_NORMALIZE_EXEC_ARGUMENTS` in configuration files
rg 'GITPOD_NORMALIZE_EXEC_ARGUMENTS' --glob '*.env' --glob '*.sh' --glob '*.yaml' --glob '*.yml'
# Search for `GITPOD_REPO_ROOT` in configuration files
rg 'GITPOD_REPO_ROOT' --glob '*.env' --glob '*.sh' --glob '*.yaml' --glob '*.yml'
Length of output: 414
The Problem/Issue/Bug
Codesniffer integration is currently not the best, so added tooling as well as ensure that the
installed_pathsset bydealerdirect/phpcodesniffer-composer-installerisn't unintentionally purged after composer has finished installing the site.How this PR Solves The Problem
Add tooling for php codesniffer's
phpcsandphpcbfscripts - which are useful for ensuring code meets certain standards (and will help with gitpod integrations like PHPStorm, which require the scripts to be present on the "host").Also ensure that
ddev phpcs --config-set installed_paths vendor/drupal/coder/coder_snifferis no longer accidentally overwritten with our script. I'd probably opt to remove this part of the code in the first place if possible if we already have the composer plugin.Manual Testing Instructions
phpcsandphpcbfcommands should now work with or without theddevprefix.Related Issue Link(s)
Release/Deployment notes
Summary by CodeRabbit
New Features
Improvements