Skip to content

Commit cc0d6a0

Browse files
authored
(dev): Update and improve linting infrastructure (valkey-io#104)
- Improve scripts (separate languages and allow fixing). - Update configuration (remove unnecessary exclusions) - Consistent versioning (don't use different versions locally and in CI). - Consistent behaviour (use same script locally and in CI) - Git hook and hook installation script. - Improved documentation Signed-off-by: currantw <taylor.curran@improving.com>
1 parent b892eee commit cc0d6a0

32 files changed

Lines changed: 436 additions & 441 deletions

.github/workflows/lint-rust/action.yml

Lines changed: 0 additions & 58 deletions
This file was deleted.

.github/workflows/php.yml

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ on:
1515
- .github/workflows/install-shared-dependencies/action.yml
1616
- .github/workflows/build-php-wrapper/action.yml
1717
- .github/workflows/test-benchmark/action.yml
18-
- .github/workflows/lint-rust/action.yml
1918
- .github/workflows/install-engine/action.yml
2019
- .github/workflows/create-test-matrices/action.yml
2120
- .github/json_matrices/**
@@ -29,7 +28,6 @@ on:
2928
- .github/workflows/install-shared-dependencies/action.yml
3029
- .github/workflows/build-php-wrapper/action.yml
3130
- .github/workflows/test-benchmark/action.yml
32-
- .github/workflows/lint-rust/action.yml
3331
- .github/workflows/install-engine/action.yml
3432
- .github/workflows/create-test-matrices/action.yml
3533
- .github/json_matrices/**
@@ -842,13 +840,13 @@ jobs:
842840
libtool \
843841
pkg-config \
844842
libssl-dev \
845-
clang-format \
846843
protobuf-c-compiler \
847844
libprotobuf-c-dev \
848845
libprotobuf-c1
849846
850-
- name: Check clang-format version
851-
run: clang-format --version
847+
# Install clang-format-18 and set it as default.
848+
sudo apt-get install -y clang-format-18
849+
sudo update-alternatives --install /usr/bin/clang-format clang-format /usr/bin/clang-format-18 100
852850
853851
- name: Install protoc
854852
uses: ./.github/workflows/install-rust-and-protoc
@@ -890,15 +888,9 @@ jobs:
890888
run: |
891889
./configure --enable-valkey-glide --disable-header-generation
892890
893-
- name: Lint C code formatting
891+
- name: Run linters
894892
run: |
895-
find . -name "*.c" -o -name "*.h" | grep -v "\.pb-c\." | xargs clang-format --dry-run --Werror
896-
897-
- name: Lint PHP code (if PHP files exist)
898-
run: |
899-
if find tests/ -name "*.php" | head -1 | grep -q .; then
900-
phpcs . || true
901-
fi
893+
./lint.sh
902894
903895
get-containers:
904896
runs-on: ubuntu-latest

.vscode/settings.json

Lines changed: 0 additions & 117 deletions
This file was deleted.

AGENTS.md

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -109,11 +109,21 @@ Use conventional commit format:
109109
110110
### Code Quality Requirements
111111
112-
**C Linters:**
113-
Run clang-format on any modified .c and .h files.
112+
#### Running Linters
114113
115-
**PHP Linters:**
116-
To be added.
114+
```bash
115+
# All linters
116+
./lint.sh # Check
117+
./lint.sh --fix # Fix
118+
119+
# C code only
120+
./lint-c.sh # Check
121+
./lint-c.sh --fix # Fix
122+
123+
# PHP code only
124+
./lint-php.sh # Check
125+
./lint-php.sh --fix # Fix
126+
```
117127

118128
## Guardrails & Policies
119129

@@ -123,13 +133,12 @@ See the .gitignore
123133
## Quality Gates (Agent Checklist)
124134

125135
- [ ] Build passes: `phpize, ./configure, make` succeeds
126-
- [ ] All tests pass: `make install` succeeds
127-
- [ ] Linting passes: `clang-format` does not report warnings on changed .c or .h files
136+
- [ ] All tests pass: `make test` succeeds
137+
- [ ] Linting passes: `./lint.sh` does not report warnings
128138
- [ ] No generated outputs committed (check `.gitignore`)
129139
- [ ] DCO signoff present: `git log --format="%B" -n 1 | grep "Signed-off-by"`
130140
- [ ] Conventional commit format used
131141
- [ ] Documentation follows Google Style format
132-
- [ ] Shared logic properly isolated in `glide-shared/`
133142

134143
## Quick Facts for Reasoners
135144

DEVELOPER.md

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,12 @@ See the [Valkey installation guide](https://valkey.io/topics/installation/) to i
4242
```bash
4343
sudo apt update -y
4444
sudo apt install -y php-dev php-cli git gcc make autotools-dev libtool pkg-config openssl libssl-dev unzip libprotobuf-c-dev libprotobuf-c1
45+
46+
# Install clang-format-18 and ensure clang-format points to it.
47+
sudo apt install -y clang-format-18
48+
sudo update-alternatives --install /usr/bin/clang-format clang-format /usr/bin/clang-format-18 100
49+
clang-format --version # Version 18.x
50+
4551
# Install rust
4652
curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh
4753
source "$HOME/.cargo/env"
@@ -74,6 +80,12 @@ curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh
7480
source "$HOME/.cargo/env"
7581
# Check that the Rust compiler is installed
7682
rustc --version
83+
84+
# Install clang-format-18 and update PATH to point to it.
85+
brew install llvm@18
86+
echo 'export PATH="/opt/homebrew/opt/llvm@18/bin:$PATH"' >> ~/.zshrc
87+
source ~/.zshrc
88+
clang-format --version # Should show version 18.x
7789
```
7890

7991
**Install protobuf compiler**
@@ -263,24 +275,33 @@ php -n -d extension=../modules/valkey_glide.so TestValkeyGlide.php
263275

264276
Development on the PHP wrapper involves changes in both C and PHP code. We have comprehensive linting infrastructure to ensure code quality and consistency. All linting checks are automatically run in our GitHub Actions CI pipeline.
265277

266-
#### Language-specific Linters
278+
- `clang-format` - C code formatting with Google-based style. Configured by `.clang-format`.
279+
- `phpcs` (PHP_CodeSniffer) - enforces PHP code standards. Configured by `phpcs.xml`.
280+
- `phpcbf` (PHP Code Beautifier and Fixer) - corrects PHP code standards. Configured by `phpcs.xml`.
267281

268-
**C Code:**
269-
- **clang-format**: Code formatting with Google-based style (4-space indentation, 100-char line limit)
270-
- **valgrind**: Memory leak detection during testing
282+
#### Running Linters
283+
284+
```bash
285+
# All linters
286+
./lint.sh # Check
287+
./lint.sh --fix # Fix
271288
272-
**PHP Code:**
273-
- **PHP_CodeSniffer (phpcs)**: coding standards.
274-
- **PHP Code Beautifier (phpcbf)**: Automatic code formatting
289+
# C code only
290+
./lint-c.sh # Check
291+
./lint-c.sh --fix # Fix
275292
293+
# PHP code only
294+
./lint-php.sh # Check
295+
./lint-php.sh --fix # Fix
296+
```
276297

277-
#### Linting Configuration Files
298+
#### Git Hooks
278299

279-
The project includes comprehensive linting configuration:
300+
Install the pre-commit hook to automatically run linters before each commit:
280301

281-
- **`phpcs.xml`**: PHP_CodeSniffer configuration with PSR-12 standards
282-
- **`.clang-format`**: C code formatting rules (Google-based style)
283-
- **`composer.json`**: Development dependencies and linting scripts
302+
```bash
303+
./git_hooks/install-git-hooks.sh
304+
```
284305

285306
#### IDE Integration
286307

@@ -291,8 +312,6 @@ VSCode is configured to automatically use the project's linting tools:
291312
- Integration with project-specific linting configurations
292313
- Extensions recommended: PHP, C/C++, phpcs, phpstan, clangd
293314
294-
295-
296315
### Extension Development Guidelines
297316
298317
#### PHP Extension Conventions

MetricsConfig.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
<?php
22

3+
declare(strict_types=1);
4+
35
namespace ValkeyGlide\OpenTelemetry;
46

57
/**

MetricsConfigBuilder.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
<?php
22

3+
declare(strict_types=1);
4+
35
namespace ValkeyGlide\OpenTelemetry;
46

57
use ValkeyGlideException;

OpenTelemetryConfig.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
<?php
22

3+
declare(strict_types=1);
4+
35
namespace ValkeyGlide\OpenTelemetry;
46

57
/**

OpenTelemetryConfigBuilder.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
<?php
22

3+
declare(strict_types=1);
4+
35
namespace ValkeyGlide\OpenTelemetry;
46

57
use ValkeyGlideException;

TracesConfig.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
<?php
22

3+
declare(strict_types=1);
4+
35
namespace ValkeyGlide\OpenTelemetry;
46

57
/**

0 commit comments

Comments
 (0)