Skip to content

Move rule code from description to check_name in GitLab output serializer#16437

Merged
MichaReiser merged 3 commits intoastral-sh:mainfrom
InSyncWithFoo:um
Feb 28, 2025
Merged

Move rule code from description to check_name in GitLab output serializer#16437
MichaReiser merged 3 commits intoastral-sh:mainfrom
InSyncWithFoo:um

Conversation

@InSyncWithFoo
Copy link
Copy Markdown
Contributor

@InSyncWithFoo InSyncWithFoo commented Feb 28, 2025 •
edited
Loading

Summary

Resolves #16435.

The changes are summarized by the following table:

Property Has rule? Before After
check_name Yes - A123
description Yes A123: Some explanation Some explanation
check_name No - syntax-error
description No SyntaxError: Some explanation Some explanation

Test Plan

cargo nextest run and cargo insta test.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 28, 2025 •
edited
Loading

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@InSyncWithFoo
Copy link
Copy Markdown
Contributor Author

InSyncWithFoo commented Feb 28, 2025 •
edited
Loading

I don't use GitLab, so this is pretty much guesswork. The description of check_name says it should be "[a] unique name representing the check, or rule, associated with this violation". Is the rule code enough, or should it be more similar to that of --output-format=github (Ruff (A123))?

@MichaReiser
Copy link
Copy Markdown
Member

MichaReiser commented Feb 28, 2025

The internal name seems fine. This is what eslint does

https://gitlab.com/remcohaszing/eslint-formatter-gitlab/-/blob/main/lib/eslint-formatter-gitlab.js?ref_type=heads#L121

@MichaReiser MichaReiser added the cli Related to the command-line interface label Feb 28, 2025
MichaReiser
MichaReiser reviewed Feb 28, 2025
View reviewed changes
Comment thread crates/ruff_linter/src/message/gitlab.rs Outdated
MichaReiser
MichaReiser approved these changes Feb 28, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

Thanks, this is great

@MichaReiser MichaReiser enabled auto-merge (squash) February 28, 2025 14:25
@MichaReiser MichaReiser merged commit 980faff into astral-sh:main Feb 28, 2025
@InSyncWithFoo InSyncWithFoo deleted the um branch February 28, 2025 14:36
@BrewTestBot BrewTestBot mentioned this pull request Mar 7, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MichaReiser MichaReiser MichaReiser approved these changes

Assignees

No one assigned

Labels

cli Related to the command-line interface

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ruff check --output-format=gitlab is missing required property "check_name"

2 participants

@InSyncWithFoo @MichaReiser