-
-
Notifications
You must be signed in to change notification settings - Fork 28
[#1911] Added markdownlint to lint docs and markdown + fixed violations. #1929
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.
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
[#1911] Added markdownlint to lint docs and markdown + fixed violations. #1929
Conversation
|
Warning Rate limit exceeded@AlexSkrypnyk has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 46 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (25)
📒 Files selected for processing (47)
WalkthroughAdds Markdown linting: new markdownlint configs and scripts, a lint.markdown.sh runner, docs lint scripts and dependency, an Ahoy command, and a CI workflow invocation; plus widespread documentation formatting edits and a VerticalTabs heading change with updated unit test. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub Actions
participant Run as ./tests/lint.markdown.sh
participant Y as Yarn (optional)
participant ML as markdownlint-cli2
Dev->>GH: push / PR triggers workflow
GH->>Run: invoke ./tests/lint.markdown.sh
alt markdownlint-cli2 not present
Run->>Y: yarn install (frozen-lockfile) in .vortex/tests
Y-->>Run: node_modules/.bin/markdownlint-cli2 available
end
Run->>ML: lint target files (docs, README, SECURITY.md, etc.)
ML-->>GH: exit code (0 pass / non-zero fail)
GH-->>Dev: report job status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 51
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (31)
.vortex/docs/content/drupal/settings.mdx (3)
69-89: Replace HTML line breaks inside list items with Markdown paragraphs for markdownlint compliance.Using
<br/>inside list items can trip markdownlint (MD033) unless explicitly allowed. Prefer plain Markdown paragraphs within list items for portability and readability.Apply this diff to remove
<br/>while preserving formatting:- - **Do not modify `settings.php` directly.<br/>** + - **Do not modify `settings.php` directly.** Use environment variables for [general](#general) settings or [per-module override files](#per-module-overrides) for any custom or environment-aware configuration. - - **Keep all overrides isolated by module.**<br/> + - **Keep all overrides isolated by module.** Each file should encapsulate logic for a single module only. If a module is removed, its override file should be removable without affecting unrelated settings. - - **Use environment variables for configuration that changes by environment.**<br/> + - **Use environment variables for configuration that changes by environment.** Prefix all such variables with `DRUPAL_` (e.g. `DRUPAL_MY_SETTING`) to easily - distinguish them from other environment variables.<br/> + distinguish them from other environment variables. Always define a default hardcoded value for each environment variable. - - **Use conditions based on _environment type_ within per-module override files.**<br/> + - **Use conditions based on _environment type_ within per-module override files.** This allows for environment-specific configuration without cluttering the main - `settings.php` file.<br/> + `settings.php` file. Use the `$settings['environment']` variable to check the current _environment type_.
90-99: Avoid duplicating the Environment indicator example block.The “Environment indicator” example appears twice (Lines 92–97 and 248–253). Keeping a single instance reduces doc length and maintenance burden. Suggest leaving it only under “Per-module overrides”.
Option A — remove the earlier block:
-import EnvironmentIndicatorModuleSettingsExample from '!!raw-loader!./../../../../web/sites/default/includes/modules/settings.environment_indicator.php'; -<details> - <summary>Example of the `Environment indicator` module settings file</summary> - -<CodeBlock language="php">{EnvironmentIndicatorModuleSettingsExample}</CodeBlock> - -</details>Option B — keep the earlier block and drop the later duplicate:
-<details> - <summary>Example of the `Environment indicator` module settings file</summary> - -<CodeBlock language="php">{EnvironmentIndicatorModuleSettingsExample}</CodeBlock> - -</details>Also applies to: 248-253
99-119: Optional: Hoist imports to the top of the MDX (style consistency).
CodeBlockis imported after its first usage (earlier section). MDX will still work, but hoisting imports just below frontmatter keeps style consistent across docs..vortex/docs/content/getting-started/faqs.mdx (1)
87-91: Fix heading grammar: “How easy is it choose …” → “How easy is it to choose …”This is user-facing and appears in navigation/anchors.
Apply this diff:
-## How easy is it choose the features that I need? +## How easy is it to choose the features that I need?.vortex/docs/src/components/VerticalTabs/VerticalTabs.js (1)
74-81: Bug: String.trim does not accept a character argument; trailing hyphens aren’t removed
trim('-')is a no-op for hyphens; use a regex to strip leading/trailing hyphens.Apply this diff:
const createSlug = title => { return title .toLowerCase() .replace(/[^\w\s-]/g, '') // Remove special characters except spaces and hyphens .replace(/\s+/g, '-') // Replace spaces with hyphens .replace(/-+/g, '-') // Replace multiple hyphens with single hyphen - .trim('-'); // Remove leading/trailing hyphens + .replace(/^-+|-+$/g, ''); // Remove leading/trailing hyphens };.vortex/docs/content/workflows/updating-vortex.mdx (1)
13-18: Clarify “Enhanced deployment capabilities” with a concrete cross-linkThis new bullet is vague. Consider linking to the Deployment workflow page or briefly enumerating what “enhanced” means (e.g., zero-downtime previews, staging auto-deploys).
For example:
-- Enhanced deployment capabilities +- Enhanced deployment capabilities (e.g., preview/staging auto-deploys). See [Deployment](/workflows/deployment)..vortex/docs/content/tools/hadolint.mdx (1)
28-33: Remove blank line so “ignore next line” example is unambiguous.For hadolint’s “ignore next line” directive, place the comment immediately above the command with no intervening blank line to avoid confusion in reader mental models.
Apply:
```Dockerfile FROM ubuntu - # hadolint ignore=DL3003,SC1035 RUN cd /tmp && echo "hello!"</blockquote></details> <details> <summary>.vortex/docs/.markdownlint.yaml (1)</summary><blockquote> `1-30`: **Solid base config; consider tuning MD024 instead of disabling.** This config is clear and well-commented. One improvement: instead of fully disabling MD024 (duplicate headings), set options that reflect MDX/doc realities (allow duplicates in different nesting). This preserves useful checks without generating noise. Suggested tweak: ```diff -# MD024/no-duplicate-heading : Multiple headings with the same content : https://github.com/DavidAnson/markdownlint/blob/v0.38.0/doc/md024.md -MD024: false +# MD024/no-duplicate-heading : Multiple headings with the same content : https://github.com/DavidAnson/markdownlint/blob/v0.38.0/doc/md024.md +# Allow duplicates when headings are in different nesting levels/sections. +MD024: + allow_different_nesting: true + siblings_only: falseOptional: If you find MD033 (inline HTML) is only needed for MDX components, keep it disabled, but document that rationale inline (helps future maintainers).
.vortex/docs/content/workflows/deployment.mdx (2)
49-57: Nice: remove trailing colon in heading and add explicit env-var example.The heading reads cleaner, and the explicit variable block improves clarity. One nit: the narrative lines (51–53) are indented by three spaces; drop the indentation for consistent rendering across MD engines.
- Suppose you want to skip the deployment for Pull Request number 42. You would - set the following environment variable: +Suppose you want to skip the deployment for Pull Request number 42. You would +set the following environment variable:
58-68: Branch “safe name” guidance is good; remove leading indentation for paragraphs.The safe-name note is helpful. As above, unindent the prose to avoid odd spacing and ensure consistent wrapping. Keep the fenced code block as-is.
- Suppose you want to skip the deployment for a branch named `feature-x`. You - would first create a "safe" version of the branch name by replacing any - special characters with underscores. +Suppose you want to skip the deployment for a branch named `feature-x`. You +would first create a "safe" version of the branch name by replacing any +special characters with underscores. - - Set the following environment variable: +Set the following environment variable:.vortex/docs/tests/unit/VerticalTabs/VerticalTabs.test.js (7)
31-39: Avoid permanently monkey-patching window event APIs; restore originals in teardown.Overriding
window.addEventListener/removeEventListenerinbeforeEachwithout restoring them risks cross-test leakage. Capture originals and restore them inafterEach/afterAll.describe('VerticalTabs with VerticalTab/VerticalTabPanel Components', () => { + const realAddEventListener = window.addEventListener; + const realRemoveEventListener = window.removeEventListener; + const realLocation = window.location; + const realHistory = window.history; beforeEach(() => { // Reset mocks before each test mockLocation.hash = ''; mockHistory.replaceState.mockClear(); // Clear any existing hashchange listeners - window.removeEventListener = jest.fn(); - window.addEventListener = jest.fn(); + window.removeEventListener = jest.fn(); + window.addEventListener = jest.fn(); }); + afterEach(() => { + window.addEventListener = realAddEventListener; + window.removeEventListener = realRemoveEventListener; + }); + afterAll(() => { + Object.defineProperty(window, 'location', { configurable: true, value: realLocation }); + Object.defineProperty(window, 'history', { configurable: true, value: realHistory }); + });
421-444: Consider asserting no history mutation on initial hash-driven selection.On mount with a matching hash, we should not rewrite history. Add a defensive assertion.
// Should show content for second tab (Docker Services) expect(screen.getByText('Docker content here')).toBeInTheDocument(); expect(screen.queryByText('Drupal content here')).not.toBeInTheDocument(); // Second tab should be active const dockerTab = screen.getByText('Docker Services').closest('.tab-item'); expect(dockerTab).toHaveClass('active'); + // No hash rewrite on initial render + expect(mockHistory.replaceState).not.toHaveBeenCalled();
474-498: Prefer dispatching a hashchange event over invoking the handler directly.Manually invoking the captured handler works but diverges from actual behavior. Dispatching an event more closely mirrors browser semantics.
- if (hashChangeHandler) { - act(() => { - hashChangeHandler(); - }); - } + act(() => { + // Fallback to a generic Event in JSDOM + window.dispatchEvent(new (window.HashChangeEvent || Event)('hashchange')); + });
516-537: Strengthen “ignores invalid hash” by asserting history is not rewritten.Verifies no accidental
replaceStateon bad hashes.// Should default to first tab when hash doesn't match any tab expect(screen.getByText('Drupal content here')).toBeInTheDocument(); expect(screen.queryByText('Docker content here')).not.toBeInTheDocument(); const drupalTab = screen.getByText('Drupal Foundation').closest('.tab-item'); expect(drupalTab).toHaveClass('active'); + expect(mockHistory.replaceState).not.toHaveBeenCalled();
540-558: Add explicit assertion that empty hash does not trigger history updates.Prevents regressions around unnecessary hash rewrites for default state.
// Should default to first tab expect(screen.getByText('Drupal content here')).toBeInTheDocument(); expect(screen.queryByText('Docker content here')).not.toBeInTheDocument(); + expect(mockHistory.replaceState).not.toHaveBeenCalled();
574-591: Slug tests look good; consider asserting call count for tighter verification.Confirms a single history update per click.
fireEvent.click(screen.getByText('Special Characters & Symbols!')); expect(mockHistory.replaceState).toHaveBeenCalledWith( null, '', '/test#special-characters-symbols' ); +expect(mockHistory.replaceState).toHaveBeenCalledTimes(1); // Click second tab to check multiple spaces handling fireEvent.click(screen.getByText('Multi Spaces Tab')); expect(mockHistory.replaceState).toHaveBeenCalledWith( null, '', '/test#multi-spaces-tab' ); +expect(mockHistory.replaceState).toHaveBeenCalledTimes(2);
681-694: Test intent mismatch: also assert URL is not updated for empty slug.The test name promises no URL update but does not verify it. Add a negative assertion.
// Should work without URL update (due to empty slug) expect(screen.getByText('Panel for empty tab')).toBeInTheDocument(); + expect(mockHistory.replaceState).not.toHaveBeenCalled();docs/deployment.md (1)
58-75: Fix ordered list structure under “does the following” (markdownlint MD029) and remove spacing artifacts.Items 4–9 should be a nested list under point 3, not a continuation at the top level. Also remove extra intra-line spaces. Proposed restructuring:
-4. Builds a website using production DB. -5. Runs code standard checks and Behat tests on the built website. -6. Creates a deployment artifact (project files to be pushed to Acquia Cloud - repository). -7. Pushes created artifact to the Acquia Cloud repository: - - for feature-based branches (i.e. `feature/ABC-123` or `bugfix/ABC-123`) - the code is pushed to the branch with exactly the same name. - - for release deployments, which are tag-based (i.e. `0.1.4`), the code is - pushed to the branch `deployment/[tag]` (i.e. `deployment/0.1.4`). -8. Acquia Cloud picks up recent push to the repository and - runs [post code update hooks](hooks/dev/post-code-update) on the environments - where code is already deployed. - OR -9. If the branch has not been deployed into any Acquia Cloud environment yet and - the developer starts the deployment, Acquia Cloud - runs [post code deploy hooks](hooks/dev/post-code-deploy) on the environment - where code is being deployed. + 1. Build the website using the production DB. + 2. Run code standard checks and Behat tests on the built website. + 3. Create a deployment artifact (project files to be pushed to the Acquia Cloud repository). + 4. Push the created artifact to the Acquia Cloud repository: + - For feature branches (e.g., `feature/ABC-123`, `bugfix/ABC-123`), push to a branch with the same name. + - For releases (tag-based, e.g., `0.1.4`), push to the branch `deployment/[tag]` (e.g., `deployment/0.1.4`). + 5. Acquia Cloud picks up the recent push to the repository and runs + [post code update hooks](hooks/dev/post-code-update) on environments where code is already deployed. + 6. If the branch has not been deployed to any Acquia Cloud environment yet and the developer starts the deployment, + Acquia Cloud runs [post code deploy hooks](hooks/dev/post-code-deploy) on the environment being deployed..vortex/docs/content/tools/twig-cs-fixer.mdx (1)
47-63: Tighten phrasing for identifier syntax, keep expanded forms.Minor editorial tweak for clarity and grammar.
-All errors have an identifier with the syntax: `A.B:C:D` with +All errors have an identifier with the syntax `A.B:C:D`, where: @@ -The four parts are optional, all those format are working +Each part is optional; the following formats are valid:.vortex/docs/content/workflows/variables.mdx (1)
826-833: Inconsistent default vs. allowed values for VORTEX_DEPLOY_ACTION.You list allowed values as deploy, deploy_override_db, destroy, but the documented default is create. Either update the default or include create in the allowed set with a description.
Apply one of the following:
Option A (change default to deploy):
-Default value: `create` +Default value: `deploy`Option B (document create as an allowed value):
-Values can be one of: deploy, deploy_override_db, destroy. +Values can be one of: deploy, deploy_override_db, destroy, create.If Option B, also add:
+- create: Create the environment (initial provisioning) without deployment..vortex/docs/content/workflows/testing.mdx (1)
96-114: Optional: Document the standard user-story format for feature files.Leaning on our prior learning, consider adding a one-liner note that BDD features should follow “As a … / I want … / So that …”.
You could append after the “Additional features include” list:
+> Convention: Write Behat features using the standard user-story format: +> "As a [user type]\nI want to [action]\nSo that [benefit]"..vortex/docs/content/tools/phpcs.mdx (1)
74-80: Typo in XML comment (“rules”).Singular noun agrees with “this … is”.
-<!-- Comment about why this rules is excluded. --> +<!-- Comment about why this rule is excluded. -->.vortex/docs/content/tools/phpunit.mdx (2)
11-11: Incorrect tool reference: PHPCS vs. PHPUnit.This page should reference the PHPUnit configuration, not PHPCS.
-**Vortex** comes with [pre-configured PHPCS ruleset](https://github.com/drevops/vortex/blob/develop/phpunit.xml) for Drupal projects. +**Vortex** comes with a [pre-configured PHPUnit configuration](https://github.com/drevops/vortex/blob/develop/phpunit.xml) for Drupal projects.
76-83: Wording tweak: “to adding” → “to add”.Minor grammar improvement.
-The recommended way to adding test targets is via using test suites: +The recommended way to add test targets is to use test suites:.vortex/docs/content/tools/docker.mdx (1)
244-248: Small UX nit: remove blank lines inside the validation code fence.Keeps the snippet compact and copy-paste friendly.
-```bash - -docker compose -f docker-compose.yml config - -``` +```bash +docker compose -f docker-compose.yml config +```.vortex/docs/content/tools/behat.mdx (2)
13-15: Grammar: add missing article (“a”)“within container environment” → “within a container environment” for correctness.
Apply this diff:
-Since Behat requires a running Drupal instance, it can only be run within -container environment. +Since Behat requires a running Drupal instance, it can only be run within +a container environment.
38-48: Use a heading instead of bold for section consistencyOther subsections use “### …”. Convert this bolded title to a level-3 heading for consistency and easier linking.
Apply this diff:
-**Running tagged tests with `@group_name` tag** +### Running tagged tests(Optional) If you want to keep the tag mention upfront, you can add a short lead-in sentence below the heading:
Use the following to run tests tagged with a specific group, for example @group_name:.vortex/docs/content/drupal/provision.mdx (2)
9-13: Grammar: add missing article (“an”)“on already assembled codebase” → “on an already assembled codebase.”
Apply this diff:
-Drupal site on already assembled codebase by either importing an existing +Drupal site on an already assembled codebase by either importing an existing
46-53: Name the exact commands for post-steps to help usersSince you now list “Post-provisioning custom scripts” and “Cache rebuilds”, consider explicitly naming the commands users will typically see (e.g.,
drush cache:rebuild) for quick scanning.Proposed tweak:
- Post-provisioning custom scripts - Cache rebuilds + Post-provisioning custom scripts (see “Running custom scripts” below) + Cache rebuilds (e.g., `drush cache:rebuild`).github/workflows/vortex-test-common.yml (1)
71-76: Add Markdown lint to Ahoy’slinttask for CI parity
The CI workflow’s “Lint code” step runs./tests/lint.markdown.sh, but the Ahoylintaggregate omits thelint-markdowncommand. To keep local and CI lint steps in sync, update the Ahoy configuration:• File:
.vortex/.ahoy.yml
• Section:linttaskApply this diff:
lint: name: Lint Vortex project. cmd: | ahoy lint-installer ahoy lint-scripts ahoy lint-dockerfiles + ahoy lint-markdown ahoy lint-docsIf you’d rather preserve a conscious difference, add a note in the project README clarifying that
ahoy lintexcludes the root Markdown lint step..vortex/.ahoy.yml (1)
40-47: Optional: include lint-markdown in the aggregate lintFor local/CI parity and a single entrypoint, consider adding
ahoy lint-markdownhere.Proposed snippet (outside current hunk; shown for clarity):
lint: name: Lint Vortex project. cmd: | ahoy lint-installer ahoy lint-scripts ahoy lint-dockerfiles ahoy lint-markdown ahoy lint-docs
| Join today! | ||
| <div align="center"> | ||
| <a href="https://drupal.slack.com/archives/CRE86HQTW"><img src="https://img.shields.io/badge/drupal.slack.com-%23DrevOps-blue.svg?logo=slack&style=for-the-badge"/></a> | ||
| <a href="https://drupal.slack.com/archives/CRE86HQTW"><img src="https://img.shields.io/badge/drupal.slack.com-%23DrevOps-blue.svg?logo=slack&style=for-the-badge" alt="Badge of the Drupal slack"/></a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Use descriptive, action-oriented alt text for the badge image
Current alt text is generic (“Badge of the Drupal slack”). Since the image is the only link content, the alt should serve as the link label for screen readers (e.g., what action/target the link represents).
Apply:
- <a href="https://drupal.slack.com/archives/CRE86HQTW"><img src="https://img.shields.io/badge/drupal.slack.com-%23DrevOps-blue.svg?logo=slack&style=for-the-badge" alt="Badge of the Drupal slack"/></a>
+ <a href="https://drupal.slack.com/archives/CRE86HQTW"><img src="https://img.shields.io/badge/drupal.slack.com-%23DrevOps-blue.svg?logo=slack&style=for-the-badge" alt="Join #DrevOps on Drupal Slack"/></a>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <a href="https://drupal.slack.com/archives/CRE86HQTW"><img src="https://img.shields.io/badge/drupal.slack.com-%23DrevOps-blue.svg?logo=slack&style=for-the-badge" alt="Badge of the Drupal slack"/></a> | |
| <a href="https://drupal.slack.com/archives/CRE86HQTW"><img src="https://img.shields.io/badge/drupal.slack.com-%23DrevOps-blue.svg?logo=slack&style=for-the-badge" alt="Join #DrevOps on Drupal Slack"/></a> |
🤖 Prompt for AI Agents
.vortex/docs/content/contributing/README.mdx around line 36: the img alt text is
generic ("Badge of the Drupal slack"); replace it with a concise, descriptive,
action-oriented label that conveys the link target and action for screen readers
(for example "Join the DrevOps channel on Drupal Slack" or "Visit DrevOps on
Drupal Slack") so the image functions as a clear link label.
| 1. Adding integration with Platform.sh. | ||
| 2. Adding support for Cypress and Drupal Test Traits. | ||
| 2. Adding support for Visual Regression and Accessibility tools. | ||
| 3. Introduction of the **Vortex** Dashboard as a single place for managing projects. | ||
| 3. Adding support for Visual Regression and Accessibility tools. | ||
| 4. Introduction of the **Vortex** Dashboard as a single place for managing projects. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Optional copy edit: prefer imperative mood and reduce repetition.
Minor editorial polish; keeps meaning while avoiding repeated “Adding” starts.
-1. Adding integration with Platform.sh.
-2. Adding support for Cypress and Drupal Test Traits.
-3. Adding support for Visual Regression and Accessibility tools.
-4. Introduction of the **Vortex** Dashboard as a single place for managing projects.
+1. Integrate with Platform.sh.
+1. Add support for Cypress and Drupal Test Traits.
+1. Add support for visual regression and accessibility tools.
+1. Introduce the **Vortex** Dashboard as a single place for managing projects.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 1. Adding integration with Platform.sh. | |
| 2. Adding support for Cypress and Drupal Test Traits. | |
| 2. Adding support for Visual Regression and Accessibility tools. | |
| 3. Introduction of the **Vortex** Dashboard as a single place for managing projects. | |
| 3. Adding support for Visual Regression and Accessibility tools. | |
| 4. Introduction of the **Vortex** Dashboard as a single place for managing projects. | |
| 1. Integrate with Platform.sh. | |
| 1. Add support for Cypress and Drupal Test Traits. | |
| 1. Add support for visual regression and accessibility tools. | |
| 1. Introduce the **Vortex** Dashboard as a single place for managing projects. |
🧰 Tools
🪛 LanguageTool
[style] ~27-~27: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... for Cypress and Drupal Test Traits. 3. Adding support for Visual Regression and Acces...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~27-~27: There might be a mistake here.
Context: ...sual Regression and Accessibility tools. 4. Introduction of the Vortex Dashboard...
(QB_NEW_EN)
🤖 Prompt for AI Agents
.vortex/docs/content/contributing/roadmap.mdx around lines 25 to 28: the
bulleted roadmap entries repeatedly start with "Adding" and use non-imperative
phrasing; change each item to concise imperative mood and remove repetition
(e.g., "Integrate with Platform.sh", "Support Cypress and Drupal Test Traits",
"Support visual regression and accessibility tools", "Introduce the Vortex
Dashboard as a single place to manage projects") ensuring consistent
capitalization and punctuation across items.
| ```bash | ||
| web/sites/default/includes/modules/settings.[module_name].php | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Use a neutral code fence language for a path-only snippet.
The snippet shows a file path, not shell commands. Using text prevents misleading Bash highlighting.
-```bash
+```text
web/sites/default/includes/modules/settings.[module_name].php
<details>
<summary>🤖 Prompt for AI Agents</summary>
.vortex/docs/content/drupal/settings.mdx around lines 237 to 239: the fenced
code block uses "bash" for a path-only snippet which misleads highlighting;
change the fence language to a neutral one such as "text" (or remove the
language) so the snippet reads text web/sites/default/includes/modules/settings.[module_name].php ensuring the
code fence shows the path as plain text rather than Bash-highlighted code.
</details>
<!-- fingerprinting:phantom:triton:chinchilla -->
<!-- This is an auto-generated comment by CodeRabbit -->
| ctools: https:/drupal.org/path/to/file.patch | ||
| ``` | ||
|
|
||
| - [ ] Assess if there are any libraries used in the project, find their |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Fix URL typo in example patch entry.
The example uses a malformed URL with a single slash after the scheme.
Apply this diff to correct the URL:
- ctools: https:/drupal.org/path/to/file.patch
+ ctools: https://drupal.org/path/to/file.patch📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ```text | |
| ctools: https:/drupal.org/path/to/file.patch | |
| ``` |
🤖 Prompt for AI Agents
In docs/onboarding.md around lines 25 to 28 the example patch URL is malformed
("https:/drupal.org/...") and missing the second slash after the scheme; update
the example to use the correct URL form by replacing
"https:/drupal.org/path/to/file.patch" with
"https://drupal.org/path/to/file.patch" so the scheme has two slashes.
| - Correct: `0.1.0`, `1.0.0` , `1.0.1` , `1.0.10` | ||
| - Incorrect: `0.1` , `1` , `1.0` , `1.0.01` , `1.0.010` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Remove spaces before commas in example lists
There are stray spaces before commas which many linters flag.
-- Correct: `0.1.0`, `1.0.0` , `1.0.1` , `1.0.10`
-- Incorrect: `0.1` , `1` , `1.0` , `1.0.01` , `1.0.010`
+- Correct: `0.1.0`, `1.0.0`, `1.0.1`, `1.0.10`
+- Incorrect: `0.1`, `1`, `1.0`, `1.0.01`, `1.0.010`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - Correct: `0.1.0`, `1.0.0` , `1.0.1` , `1.0.10` | |
| - Incorrect: `0.1` , `1` , `1.0` , `1.0.01` , `1.0.010` | |
| - Correct: `0.1.0`, `1.0.0`, `1.0.1`, `1.0.10` | |
| - Incorrect: `0.1`, `1`, `1.0`, `1.0.01`, `1.0.010` |
🧰 Tools
🪛 LanguageTool
[grammar] ~36-~36: There might be a mistake here.
Context: ... Examples: - Correct: 0.1.0, 1.0.0 , 1.0.1 , 1.0.10 - Incorrect: 0.1 ...
(QB_NEW_EN_OTHER)
[grammar] ~36-~36: There might be a mistake here.
Context: ...: - Correct: 0.1.0, 1.0.0 , 1.0.1 , 1.0.10 - Incorrect: 0.1 , 1 , `1...
(QB_NEW_EN_OTHER)
[grammar] ~36-~36: There might be a mistake here.
Context: ...- Correct: 0.1.0, 1.0.0 , 1.0.1 , 1.0.10 - Incorrect: 0.1 , 1 , 1.0 , `1.0.01...
(QB_NEW_EN)
[grammar] ~37-~37: There might be a mistake here.
Context: ... , 1.0.1 , 1.0.10 - Incorrect: 0.1 , 1 , 1.0 , 1.0.01 , 1.0.010 ##...
(QB_NEW_EN_OTHER)
[grammar] ~37-~37: There might be a mistake here.
Context: ...0.1,1.0.10- Incorrect:0.1,1,1.0,1.0.01,1.0.010` ## Versi...
(QB_NEW_EN_OTHER)
[grammar] ~37-~37: There might be a mistake here.
Context: ...1.0.10- Incorrect:0.1,1,1.0,1.0.01,1.0.010` ## Version Numbe...
(QB_NEW_EN_OTHER)
[grammar] ~37-~37: There might be a mistake here.
Context: ...ncorrect: 0.1 , 1 , 1.0 , 1.0.01 , 1.0.010 ## Version Number - CalVer ...
(QB_NEW_EN_OTHER)
[grammar] ~37-~37: Use correct spacing
Context: ...rrect: 0.1 , 1 , 1.0 , 1.0.01 , 1.0.010 ## Version Number - CalVer Release version...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🤖 Prompt for AI Agents
In docs/releasing.md around lines 36 to 37, the example lists have stray spaces
before commas (e.g. "`0.1.0` , `1.0.0`") which linters flag; remove the space
immediately before each comma so commas directly follow the preceding item and
keep a single space after the comma (e.g. "`0.1.0`, `1.0.0`"), then save and run
the linter to verify no spacing errors remain.
| - `YY` = Short year. No leading zeroes. | ||
| - `M` = Short month. No leading zeroes. | ||
| - `Z` = Hotfix/patch version. No leading zeroes. | ||
|
|
||
| Examples: | ||
|
|
||
| * Correct: `24.1.0`, `24.11.1` , `24.1.10`, `24.0.1`, `9.12.0` | ||
| * Incorrect: `24.0.0`, `2024.1.1` , `24` , `24.1.00` , `24.01.0` , `01.1.0` | ||
| - Correct: `24.1.0`, `24.11.1` , `24.1.10`, `24.0.1`, `9.12.0` | ||
| - Incorrect: `24.0.0`, `2024.1.1` , `24` , `24.1.00` , `24.01.0` , `01.1.0` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CalVer example “24.0.1” contradicts rule “M = Short month. No leading zeroes.” — month 0 is invalid
Replace 24.0.1 with a valid month (1–12) and remove extra spaces before commas in both lists.
-Given a version number `YY.M.Z`:
-
-- `YY` = Short year. No leading zeroes.
-- `M` = Short month. No leading zeroes.
-- `Z` = Hotfix/patch version. No leading zeroes.
+Given a version number `YY.M.Z`:
+
+- `YY` = Short year. No leading zeroes.
+- `M` = Short month (1–12). No leading zeroes.
+- `Z` = Hotfix/patch version. No leading zeroes.
@@
-Examples:
-
-- Correct: `24.1.0`, `24.11.1` , `24.1.10`, `24.0.1`, `9.12.0`
-- Incorrect: `24.0.0`, `2024.1.1` , `24` , `24.1.00` , `24.01.0` , `01.1.0`
+Examples:
+
+- Correct: `24.1.0`, `24.11.1`, `24.1.10`, `24.10.1`, `9.12.0`
+- Incorrect: `24.0.0`, `2024.1.1`, `24`, `24.1.00`, `24.01.0`, `01.1.0`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - `YY` = Short year. No leading zeroes. | |
| - `M` = Short month. No leading zeroes. | |
| - `Z` = Hotfix/patch version. No leading zeroes. | |
| Examples: | |
| * Correct: `24.1.0`, `24.11.1` , `24.1.10`, `24.0.1`, `9.12.0` | |
| * Incorrect: `24.0.0`, `2024.1.1` , `24` , `24.1.00` , `24.01.0` , `01.1.0` | |
| - Correct: `24.1.0`, `24.11.1` , `24.1.10`, `24.0.1`, `9.12.0` | |
| - Incorrect: `24.0.0`, `2024.1.1` , `24` , `24.1.00` , `24.01.0` , `01.1.0` | |
| Given a version number `YY.M.Z`: | |
| - `YY` = Short year. No leading zeroes. | |
| - `M` = Short month (1–12). No leading zeroes. | |
| - `Z` = Hotfix/patch version. No leading zeroes. | |
| Examples: | |
| - Correct: `24.1.0`, `24.11.1`, `24.1.10`, `24.10.1`, `9.12.0` | |
| - Incorrect: `24.0.0`, `2024.1.1`, `24`, `24.1.00`, `24.01.0`, `01.1.0` |
🧰 Tools
🪛 LanguageTool
[grammar] ~46-~46: Use correct spacing
Context: ...= Short year. No leading zeroes. -M= Short month. No leading zeroes. -Z` ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~47-~47: Use correct spacing
Context: ... = Short month. No leading zeroes. - Z = Hotfix/patch version. No leading zeroe...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~47-~47: Use correct spacing
Context: ...Hotfix/patch version. No leading zeroes. Examples: - Correct: 24.1.0, `24.11.1...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~49-~49: Use correct spacing
Context: ...h version. No leading zeroes. Examples: - Correct: 24.1.0, 24.11.1 , 24.1.10...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~51-~51: There might be a mistake here.
Context: ...xamples: - Correct: 24.1.0, 24.11.1 , 24.1.10, 24.0.1, 9.12.0 - Incorr...
(QB_NEW_EN_OTHER)
[grammar] ~51-~51: There might be a mistake here.
Context: ....1.0, 24.11.1,24.1.10, 24.0.1, 9.12.0- Incorrect:24.0.0, 2024.1.1,24` ,...
(QB_NEW_EN)
[grammar] ~52-~52: There might be a mistake here.
Context: ....12.0- Incorrect:24.0.0, 2024.1.1,24,24.1.00,24.01.0,01.1.0...
(QB_NEW_EN_OTHER)
[grammar] ~52-~52: There might be a mistake here.
Context: ...- Incorrect: 24.0.0, 2024.1.1 , 24 , 24.1.00 , 24.01.0 , 01.1.0
(QB_NEW_EN_OTHER)
[grammar] ~52-~52: There might be a mistake here.
Context: ... 24.0.0, 2024.1.1 , 24 , 24.1.00 , 24.01.0 , 01.1.0
(QB_NEW_EN_OTHER)
[grammar] ~52-~52: There might be a mistake here.
Context: ...2024.1.1,24,24.1.00,24.01.0,01.1.0`
(QB_NEW_EN_OTHER)
[grammar] ~52-~52: Use correct spacing
Context: ...4.1.1,24,24.1.00,24.01.0,01.1.0`
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🤖 Prompt for AI Agents
In docs/releasing.md around lines 45 to 52, the CalVer example "24.0.1" violates
the rule "M = Short month. No leading zeroes." — month 0 is invalid; replace
that example with a valid month between 1 and 12 (e.g., 24.1.1 or 24.10.1) and
remove the extra spaces before commas in both the Correct and Incorrect example
lists so punctuation spacing is consistent.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1929 +/- ##
========================================
Coverage 78.69% 78.69%
========================================
Files 85 85
Lines 5027 5027
Branches 35 35
========================================
Hits 3956 3956
Misses 1071 1071 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review continued from previous batch...
| :::warning | ||
|
|
||
| Sanitization takes place only after the database is **imported**, so anyone | ||
| with access to the dump file can still see sensitive data. | ||
| Sanitization takes place only after the database is **imported**, so anyone | ||
| with access to the dump file can still see sensitive data. | ||
|
|
||
| If your database has highly sensitive data, consider sanitizing the database | ||
| dump before it can be downloaded (sanitize on export). There are tools | ||
| available for this purpose, such as [Drush GDPR Dumper](https://github.com/robiningelbrecht/drush-gdpr-dumper) | ||
| or [MTK](https://github.com/skpr/mtk). | ||
| If your database has highly sensitive data, consider sanitizing the database | ||
| dump before it can be downloaded (sanitize on export). There are tools | ||
| available for this purpose, such as [Drush GDPR Dumper](https://github.com/robiningelbrecht/drush-gdpr-dumper) | ||
| or [MTK](https://github.com/skpr/mtk). | ||
|
|
||
| ::: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Security note reads well; add one guardrail
The warning is good. Consider explicitly advising teams to restrict access to dumps and use per-environment storage controls (e.g., CI artifacts retention, bucket ACLs).
Suggested addition after the warning:
Additionally, restrict access to database dumps (e.g., CI artifact retention policies, scoped storage buckets with least-privilege ACLs) to minimize exposure risk before sanitization.🧰 Tools
🪛 LanguageTool
[grammar] ~111-~111: Use correct spacing
Context: ...ronments safer to work with. :::warning Sanitization takes place only after the ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~113-~113: There might be a mistake here.
Context: ... the database is imported, so anyone with access to the dump file can still s...
(QB_NEW_EN)
[grammar] ~114-~114: Use correct spacing
Context: ... dump file can still see sensitive data. If your database has highly sensitive da...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~117-~117: There might be a mistake here.
Context: ...ed (sanitize on export). There are tools available for this purpose, such as [Dru...
(QB_NEW_EN)
[grammar] ~118-~118: There might be a mistake here.
Context: ....com/robiningelbrecht/drush-gdpr-dumper) or MTK. ...
(QB_NEW_EN)
[grammar] ~119-~119: Use correct spacing
Context: ...) or MTK. ::: The database sanitization step is e...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~121-~121: Use correct spacing
Context: ...MTK. ::: The database sanitization step is enable...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🤖 Prompt for AI Agents
.vortex/docs/content/drupal/provision.mdx around lines 111 to 121: add a
single-sentence guardrail after the existing warning advising teams to restrict
access to database dumps prior to sanitization; specifically, instruct to
mention enforcing CI artifact retention policies and using scoped storage
buckets with least-privilege ACLs (or equivalent per-environment controls) to
minimize exposure risk before sanitization.
| :::tip | ||
|
|
||
| JavaScript tests are slow. Try using as few JavaScript tests as possible to | ||
| keep the test run time low. | ||
| JavaScript tests are slow. Try using as few JavaScript tests as possible to | ||
| keep the test run time low. | ||
|
|
||
| ::: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Tighten the tip phrasing
Slightly rephrase for flow and remove redundancy.
Apply this diff:
-:::tip
-
-JavaScript tests are slow. Try using as few JavaScript tests as possible to
-keep the test run time low.
-
-:::
+:::tip
+JavaScript tests are slower. Prefer keeping the number of JS tests to a minimum to reduce overall runtime.
+:::📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| :::tip | |
| JavaScript tests are slow. Try using as few JavaScript tests as possible to | |
| keep the test run time low. | |
| JavaScript tests are slow. Try using as few JavaScript tests as possible to | |
| keep the test run time low. | |
| ::: | |
| :::tip | |
| JavaScript tests are slower. Prefer keeping the number of JS tests to a minimum to reduce overall runtime. | |
| ::: |
🧰 Tools
🪛 LanguageTool
[grammar] ~170-~170: Use correct spacing
Context: ...r tests that require JavaScript. :::tip JavaScript tests are slow. Try using as ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~172-~172: There might be a mistake here.
Context: ...g as few JavaScript tests as possible to keep the test run time low. ::: ### `F...
(QB_NEW_EN)
[grammar] ~173-~173: Use correct spacing
Context: ... possible to keep the test run time low. ::: ### FeatureContext.php and exampl...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~175-~175: Use correct spacing
Context: ...ible to keep the test run time low. ::: ### FeatureContext.php and example tests The [`FeatureContext...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🤖 Prompt for AI Agents
In .vortex/docs/content/tools/behat.mdx around lines 170–175, the tip phrasing
is redundant and slightly awkward; replace the existing multi-sentence tip with
a tighter, single-sentence version that conveys the same meaning (e.g., a
concise instruction to minimize JavaScript tests because they run slower and
increase test run time) so the tip flows better and removes repetition.
| and click on the link next to `Selenium VNC URL on host` to open the browser. | ||
|
|
||
| Or use the following command to get the port number: | ||
|
|
||
| ```shell | ||
| docker compose port chrome 7900 | ||
| ``` | ||
|
|
||
| and open `http://localhost:<port>/?autoconnect=1&password=secret` in the browser. | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Clarify the noVNC step and the placeholder
Small clarity/readability improvements for users new to the feature.
Apply this diff:
-and click on the link next to `Selenium VNC URL on host` to open the browser.
+and click the link next to `Selenium VNC URL on host` to open the noVNC browser.
-Or use the following command to get the port number:
+Alternatively, get the port number with:
@@
-and open `http://localhost:<port>/?autoconnect=1&password=secret` in the browser.
+then open `http://localhost:<port>/?autoconnect=1&password=secret` in your host browser
+(replace `<port>` with the value printed by the command).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| and click on the link next to `Selenium VNC URL on host` to open the browser. | |
| Or use the following command to get the port number: | |
| ```shell | |
| docker compose port chrome 7900 | |
| ``` | |
| and open `http://localhost:<port>/?autoconnect=1&password=secret` in the browser. | |
| and click the link next to `Selenium VNC URL on host` to open the noVNC browser. | |
| Alternatively, get the port number with: | |
🧰 Tools
🪛 LanguageTool
[grammar] ~205-~205: Use correct spacing
Context: ...um VNC URL on host` to open the browser. Or use the following command to get the ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~207-~207: Use correct spacing
Context: ...ollowing command to get the port number: shell docker compose port chrome 7900 and open `http://localhost:/?autoc...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~213-~213: Use correct spacing
Context: ...nnect=1&password=secret` in the browser. :::tip When running tests, you can add ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🤖 Prompt for AI Agents
In .vortex/docs/content/tools/behat.mdx around lines 205–214, the noVNC step is
ambiguous because it uses the raw placeholder "<port>" and terse wording; update
the text to explicitly tell users to run the docker compose port chrome 7900
command, copy the numeric port it outputs, and replace the <port> placeholder in
the noVNC URL with that number (preserve the ?autoconnect=1&password=secret
query string), and rephrase the sentence to clearly instruct "open
http://localhost:YOUR_PORT/?autoconnect=1&password=secret in your browser" so
newcomers understand they must substitute the returned port value.
| used to run Behat tests. It is based on the official Selenium image which has | ||
| additional required tools, like virtual desktop, installed. | ||
| - `wait_dependencies` - a container that runs a script to wait for applications |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Tiny article/clarity tweak in chrome service bullet.
Optional grammar polish.
-- `chrome` - container that runs a Chrome browser. This container is
- used to run Behat tests. It is based on the official Selenium image which has
- additional required tools, like virtual desktop, installed.
+- `chrome` - a container that runs a Chrome browser. This container is used to run Behat tests. It is based on the official Selenium image, which has additional required tools, like a virtual desktop, installed.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| used to run Behat tests. It is based on the official Selenium image which has | |
| additional required tools, like virtual desktop, installed. | |
| - `wait_dependencies` - a container that runs a script to wait for applications | |
| - `chrome` - a container that runs a Chrome browser. This container is used to run Behat tests. It is based on the official Selenium image, which has additional required tools, like a virtual desktop, installed. | |
| - `wait_dependencies` - a container that runs a script to wait for applications |
🧰 Tools
🪛 LanguageTool
[grammar] ~223-~223: There might be a mistake here.
Context: ...s. It is based on the official Selenium image which has additional required tools, ...
(QB_NEW_EN_OTHER)
[grammar] ~223-~223: Use correct spacing
Context: ...on the official Selenium image which has additional required tools, like virtual ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~224-~224: There might be a problem here.
Context: ... tools, like virtual desktop, installed. - wait_dependencies - a container that runs a script to wait...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~225-~225: Use correct spacing
Context: ...t runs a script to wait for applications within other containers to become availa...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🤖 Prompt for AI Agents
.vortex/docs/content/tools/docker.mdx around lines 223 to 225: the chrome
service bullet for `wait_dependencies` is slightly unclear/awkwardly phrased;
replace the current line with a clearer, grammatical sentence such as:
"`wait_dependencies` - a container that runs a script to wait for application
dependencies to become available." Ensure punctuation and backtick formatting
match the surrounding bullet style.
| Run checks against platform version specified in `composer.json` key `config.platform.php`: | ||
|
|
||
| ```xml | ||
| <config name="testVersion" value="8.1"/> | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove misplaced PHPCS config from PHPUnit docs.
The example belongs to PHPCS (PHPCompatibility), not PHPUnit. It’s misleading here.
Option A — remove the section:
-Run checks against platform version specified in `composer.json` key `config.platform.php`:
-
-```xml
-<config name="testVersion" value="8.1"/>
-```Option B — replace with accurate guidance:
-Run checks against platform version specified in `composer.json` key `config.platform.php`:
-
-```xml
-<config name="testVersion" value="8.1"/>
-```
+PHPUnit runs with the PHP version of your runtime. To standardize PHP across tools locally and in CI, set `"config.platform.php"` in `composer.json` (this affects dependency resolution, not PHPUnit itself).🧰 Tools
🪛 LanguageTool
[grammar] ~85-~85: There might be a mistake here.
Context: ...irectory> ``` Run checks against platform version specified in `composer...
(QB_NEW_EN)
[grammar] ~85-~85: Use articles correctly
Context: ...ecks against platform version specified in composer.json key `config.platform.ph...
(QB_NEW_EN_OTHER_ERROR_IDS_11)
[grammar] ~85-~85: Use correct spacing
Context: ...omposer.jsonkeyconfig.platform.php`: xml <config name="testVersion" value="8.1"/> ## Coverage PHPUnit is configured to gener...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🤖 Prompt for AI Agents
.vortex/docs/content/tools/phpunit.mdx around lines 85 to 89: the snippet
showing <config name="testVersion" value="8.1"/> is PHPCS/PHPCompatibility
config and is misplaced in the PHPUnit docs; remove that example and either
delete the whole misleading paragraph (Option A) or replace it with correct
guidance that PHPUnit uses the runtime PHP and recommend setting
"config.platform.php" in composer.json to standardize PHP for dependency
resolution and CI (Option B). Ensure the final text does not include the PHPCS
<config name="testVersion"> example and instead clearly states that PHPUnit runs
with the runtime PHP and points readers to composer.json "config.platform.php"
for tool/CI consistency.
| "lint": "yarn lint-js && yarn lint-docs-fix", | ||
| "lint-fix": "yarn lint-js-fix && yarn lint-docs-fix", | ||
| "lint-js": "eslint src/ && prettier --check src/", | ||
| "lint-js-fix": "eslint src/ --fix && prettier --write src/", | ||
| "lint-docs": "markdownlint-cli2 content/**/*.mdx", | ||
| "lint-docs-fix": "markdownlint-cli2 content/**/*.mdx --fix" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Don’t auto-fix in the default “lint” target
Running “--fix” under the default lint target can mask violations locally and in CI by mutating files instead of failing. Reserve auto-fixes for “lint-fix” and keep “lint” read-only.
Apply this diff:
- "lint": "yarn lint-js && yarn lint-docs-fix",
+ "lint": "yarn lint-js && yarn lint-docs",This keeps:
- lint: non-mutating checks only
- lint-fix: applies fixes as requested
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "lint": "yarn lint-js && yarn lint-docs-fix", | |
| "lint-fix": "yarn lint-js-fix && yarn lint-docs-fix", | |
| "lint-js": "eslint src/ && prettier --check src/", | |
| "lint-js-fix": "eslint src/ --fix && prettier --write src/", | |
| "lint-docs": "markdownlint-cli2 content/**/*.mdx", | |
| "lint-docs-fix": "markdownlint-cli2 content/**/*.mdx --fix" | |
| "lint": "yarn lint-js && yarn lint-docs", | |
| "lint-fix": "yarn lint-js-fix && yarn lint-docs-fix", | |
| "lint-js": "eslint src/ && prettier --check src/", | |
| "lint-js-fix": "eslint src/ --fix && prettier --write src/", | |
| "lint-docs": "markdownlint-cli2 content/**/*.mdx", | |
| "lint-docs-fix": "markdownlint-cli2 content/**/*.mdx --fix" |
🤖 Prompt for AI Agents
.vortex/docs/package.json lines 19-24: the "lint" script currently runs mutating
fix commands; update it to only run non-mutating checks and move all
--fix/--write steps into "lint-fix". Concretely, change "lint" to run the
check-only commands (yarn lint-js and yarn lint-docs) where lint-js is eslint +
prettier --check and lint-docs is markdownlint-cli2 checks, and ensure
"lint-fix" remains the command that runs eslint --fix, prettier --write and
markdownlint-cli2 --fix so auto-fixes are only performed by lint-fix.
| "lint-docs": "markdownlint-cli2 content/**/*.mdx", | ||
| "lint-docs-fix": "markdownlint-cli2 content/**/*.mdx --fix" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Consider limiting MDX globbing to avoid double-processing vendor/build dirs
If Docusaurus or tooling ever introduces output folders under content, the broad glob may catch generated files. Consider scoping or excluding known build/cache dirs.
Example:
- "lint-docs": "markdownlint-cli2 content/**/*.mdx",
- "lint-docs-fix": "markdownlint-cli2 content/**/*.mdx --fix"
+ "lint-docs": "markdownlint-cli2 \"content/**/*.mdx\" \"!content/**/node_modules/**\" \"!content/**/.docusaurus/**\"",
+ "lint-docs-fix": "markdownlint-cli2 \"content/**/*.mdx\" \"!content/**/node_modules/**\" \"!content/**/.docusaurus/**\" --fix"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "lint-docs": "markdownlint-cli2 content/**/*.mdx", | |
| "lint-docs-fix": "markdownlint-cli2 content/**/*.mdx --fix" | |
| "lint-docs": "markdownlint-cli2 \"content/**/*.mdx\" \"!content/**/node_modules/**\" \"!content/**/.docusaurus/**\"", | |
| "lint-docs-fix": "markdownlint-cli2 \"content/**/*.mdx\" \"!content/**/node_modules/**\" \"!content/**/.docusaurus/**\" --fix" |
🤖 Prompt for AI Agents
.vortex/docs/package.json around lines 23-24: the current MDX glob is too broad
and may double-process generated/vendor/build files; narrow the glob or add
explicit ignores for common output/cache dirs (for example restrict to
content/**/*.{md,mdx} under source directories or add --ignore patterns for
.docusaurus, build, dist, node_modules, vendor, .cache) so the linter only
targets authored MDX and skips generated artifacts.
df82cb7 to
cdf91e4
Compare
cdf91e4 to
fe8008b
Compare
Closes #1911
Summary by CodeRabbit
New Features
Documentation
Chores
Tests