Skip to content

Commit f2612dc

Browse files
josecelanojosecelano
committed
docs(issue-523): add internal linting implementation plan
1 parent 1ef5b41 commit f2612dc

1 file changed

Lines changed: 141 additions & 0 deletions

File tree

‎docs/issues/523-internal-linting-tool.md‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
# Issue #523 Implementation Plan (Internal Linting Tool)
2+
3+
## Goal
4+
5+
Replace the MegaLinter idea with Torrust internal linting tooling and integrate it into CI for this repository.
6+
7+
## Scope
8+
9+
- Target issue: https://github.com/torrust/torrust-tracker/issues/523
10+
- CI workflow to modify: .github/workflows/testing.yaml
11+
- External reference workflow: https://raw.githubusercontent.com/torrust/torrust-tracker-deployer/refs/heads/main/.github/workflows/linting.yml
12+
13+
## Tasks
14+
15+
### 0) Create a local branch following GitHub branch naming conventions
16+
17+
- Approved branch name: `523-internal-linting-tool`
18+
- Commands:
19+
- `git fetch --all --prune`
20+
- `git checkout develop`
21+
- `git pull --ff-only`
22+
- `git checkout -b 523-internal-linting-tool`
23+
- Checkpoint:
24+
- `git branch --show-current` should output `523-internal-linting-tool`.
25+
26+
### 1) Install and run the linting tool locally; verify it passes in this repo
27+
28+
- Identify/install internal linting package/tool used by Torrust (likely `torrust-linting` or equivalent wrapper).
29+
- Ensure local runtime dependencies are present (if any).
30+
- Note: linter config files (step 2) must exist in the repo root before a full suite run; it is fine to do a first exploratory run first to discover which linters are active.
31+
- Run the internal linting command against this repository.
32+
- Capture the exact command and output summary for reproducibility.
33+
- Checkpoint:
34+
- Linting command exits with code `0`.
35+
36+
### 2) Add and adapt linter configuration files
37+
38+
Some linters require a config file in the repo root. Use the deployer configs as reference and adapt values to this repository.
39+
40+
| File | Linter | Reference |
41+
| -------------------- | ---------------- | ----------------------------------------------------------------------------------------------------- |
42+
| `.markdownlint.json` | markdownlint | https://raw.githubusercontent.com/torrust/torrust-tracker-deployer/refs/heads/main/.markdownlint.json |
43+
| `.taplo.toml` | taplo (TOML fmt) | https://raw.githubusercontent.com/torrust/torrust-tracker-deployer/refs/heads/main/.taplo.toml |
44+
| `.yamllint-ci.yml` | yamllint | https://raw.githubusercontent.com/torrust/torrust-tracker-deployer/refs/heads/main/.yamllint-ci.yml |
45+
46+
Key adaptations to make per file:
47+
48+
- `.markdownlint.json`: review line-length rules and Markdown conventions used in this repo's docs.
49+
- `.taplo.toml`: update `exclude` list to match this repo's generated/runtime folders (e.g. `target/**`, `storage/**`) instead of the deployer-specific ones (`build/**`, `data/**`, `envs/**`).
50+
- `.yamllint-ci.yml`: update `ignore` block to reflect this repo's generated/runtime directories instead of cloud-init and deployer folders.
51+
52+
Commit message: `ci(lint): add linter config files (.markdownlint.json, .taplo.toml, .yamllint-ci.yml)`
53+
54+
Checkpoint:
55+
56+
- Config files are present in the repo root.
57+
- Running each individual linter against the repo with the config produces expected/controlled output.
58+
59+
### 3) If local linting fails, fix all lint errors; commit fixes independently per linter
60+
61+
- If the linting suite reports failures:
62+
- Group findings by linter (for example: formatting, clippy, docs, spelling, yaml, etc.).
63+
- Fix only one linter category at a time.
64+
- Create one commit per linter category.
65+
- Commit style proposal:
66+
- `fix(lint/<linter-name>): resolve <brief issue summary>`
67+
- Constraints:
68+
- Do not mix workflow/tooling changes with source lint fixes in the same commit.
69+
- Keep each commit minimal and reviewable.
70+
- Checkpoint:
71+
- Re-run linting suite; all checks pass before moving to workflow integration.
72+
73+
### 4) Review existing workflow example using internal linting
74+
75+
- Read and analyze:
76+
- https://raw.githubusercontent.com/torrust/torrust-tracker-deployer/refs/heads/main/.github/workflows/linting.yml
77+
- Extract and adapt:
78+
- Trigger strategy.
79+
- Tool setup/install method.
80+
- Cache strategy.
81+
- Invocation command and CI fail behavior.
82+
- Checkpoint:
83+
- Document a short mapping from deployer workflow pattern to this repo’s `testing.yaml` job structure.
84+
85+
### 5) Modify `.github/workflows/testing.yaml` to use the internal linting tool
86+
87+
- Update the current `check`/lint-related section to run the internal linting command.
88+
- Replace existing lint/check execution path with the internal linting tool in this migration (no parallel transition mode).
89+
- Ensure matrix/toolchain compatibility is explicit (nightly/stable behavior decided and documented).
90+
- Validate workflow syntax before commit.
91+
- Checkpoint:
92+
- Workflow is valid and executes linting through internal tool.
93+
94+
### 6) Commit workflow changes
95+
96+
- Commit only workflow-related changes in a dedicated commit.
97+
- Commit message proposal:
98+
- `ci(lint): switch testing workflow to internal linting tool`
99+
- Checkpoint:
100+
- `git show --name-only --stat HEAD` includes only expected workflow files (and any required supporting CI files if intentionally added).
101+
102+
### 7) Push to remote `josecelano` and open PR into `develop`
103+
104+
- Verify remote exists:
105+
- `git remote -v`
106+
- Push branch:
107+
- `git push -u josecelano 523-internal-linting-tool`
108+
- Open PR targeting `torrust/torrust-tracker:develop` with head `josecelano:523-internal-linting-tool`.
109+
- PR content should include:
110+
- Why internal linting over MegaLinter.
111+
- Summary of lint-fix commits by linter.
112+
- Summary of workflow change.
113+
- Evidence (local run + CI status).
114+
- Checkpoint:
115+
- PR is open, linked to issue #523, and ready for review.
116+
117+
## Execution Notes
118+
119+
- Keep PR review-friendly by separating commits by concern:
120+
1. Linter config files (step 2)
121+
2. Per-linter source fixes (step 3, only if needed)
122+
3. CI workflow migration (step 6)
123+
- Use Conventional Commits for all commits in this implementation.
124+
- If lint checks differ between local and CI, align tool versions and execution flags before merging.
125+
- Avoid broad refactors unrelated to lint failures.
126+
127+
## Decisions Confirmed
128+
129+
1. Branch name: `523-internal-linting-tool`.
130+
2. CI strategy: replace existing lint/check path with internal linting.
131+
3. Commit convention: yes, use Conventional Commits.
132+
4. PR target: base `torrust/torrust-tracker:develop`, head `josecelano:523-internal-linting-tool`.
133+
134+
## Risks and Mitigations
135+
136+
- Risk: Internal linting wrapper may not be version-pinned and may produce unstable CI behavior.
137+
- Mitigation: Pin tool version in workflow installation step.
138+
- Risk: Internal linting may overlap with existing checks, increasing CI time.
139+
- Mitigation: Remove redundant jobs only after verifying coverage parity.
140+
- Risk: Tool may require secrets or environment assumptions not available in CI.
141+
- Mitigation: Run dry-run in GitHub Actions on branch before requesting review.

0 commit comments

Comments
 (0)