Skip to content

Conversation

@ematipico
Copy link
Member

@ematipico ematipico commented Oct 22, 2025

Summary

Created a new workflow meant to lint the docs of our rules. They are different workflows, so they are decoupled from linting, and they run only when we change files in the analyze crates.

Test Plan

Docs

@changeset-bot
Copy link

changeset-bot bot commented Oct 22, 2025

⚠️ No Changeset found

Latest commit: 0158ffc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@ematipico ematipico requested review from a team October 22, 2025 16:38
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 22, 2025

Walkthrough

The pull request workflow has been restructured to separate linting and rules checking. The cargo lint step now runs independently with continue-on-error: true enabled, permitting lint failures without halting the workflow. A new dedicated step executes cargo run -p rules_check as a separate process. This decouples the two validation checks and modifies error handling so failures in linting don't immediately block subsequent steps.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "ci: additional step for rules check" directly aligns with the main change in the changeset. The raw summary confirms that a separate step for executing rules_check has been added to the PR workflow, decoupled from the cargo lint command. The title is concise, specific, and clearly communicates the primary modification without unnecessary noise.
Description Check ✅ Passed The PR description is related to the changeset and explains the motivation behind the changes. It describes splitting the scripts and adding continue-on-error to address the issue where rules checking was skipped when linting failed, which matches the modifications documented in the raw summary. Whilst brief, the description provides sufficient context about the improvement being made.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ci/linting-on-pr

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
.github/workflows/pull_request.yml (1)

48-51: Minor: step name doesn't match the actual command.

Line 48 labels this "Run clippy", but line 49 runs cargo lint. If cargo lint is a wrapper that includes clippy, consider renaming the step to "Run lint" for clarity.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 303eec0 and 9327935.

📒 Files selected for processing (1)
  • .github/workflows/pull_request.yml (1 hunks)
🔇 Additional comments (1)
.github/workflows/pull_request.yml (1)

49-53: Verify intended workflow behaviour with continue-on-error.

With this setup, the job will pass if cargo lint fails but cargo run -p rules_check succeeds (since only the lint step has continue-on-error: true). Confirm this is the desired behaviour—whether you want both checks to gate the job or if lint failures should be non-blocking warnings while rules_check failure blocks the job.

@ematipico ematipico changed the title ci: additional step for rules check ci: additional workflow for rules check Oct 23, 2025
@ematipico ematipico requested a review from dyc3 October 23, 2025 09:33
@ematipico
Copy link
Member Author

@dyc3 PR updated, as we as the description

@ematipico ematipico requested a review from a team October 23, 2025 09:34
Copy link
Contributor

@dyc3 dyc3 left a comment

Choose a reason for hiding this comment

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

Yeah this is definitely easier :)

@dyc3 dyc3 merged commit 36ae474 into main Oct 23, 2025
4 checks passed
@dyc3 dyc3 deleted the ci/linting-on-pr branch October 23, 2025 19:49
Jagget pushed a commit to Jagget/biome that referenced this pull request Oct 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants