Skip to content

GitLab : Code Quality report format#357

Merged
VincentLanglet merged 3 commits intoVincentLanglet:mainfrom
AlexisPPLIN:add-gitlab-reporter
Apr 27, 2025
Merged

GitLab : Code Quality report format#357
VincentLanglet merged 3 commits intoVincentLanglet:mainfrom
AlexisPPLIN:add-gitlab-reporter

Conversation

@AlexisPPLIN
Copy link
Copy Markdown
Contributor

Hi,

I started to implement this wonderful tool in my GitLab CI/CD pipeline at work.
So I implemented the Code Quality report format : https://docs.gitlab.com/ci/testing/code_quality/#code-quality-report-format

I also added unit tests as asked.

I already used this reporter for a few months with no issues.

Have a nice day / night.

*/
private function generateFingerprint(string $relativePath, Violation $violation): string
{
$base = $relativePath.$violation->getRuleName().$violation->getMessage();
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

There is a violationId

You should try something like

$id = $violation->getIdentifier()?->toString() ?? $violation->getRuleName().$violation->getMessage()<
$hash = md5($relativePath.$id);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is intended, GitLab fingerprint is a bit weird.
See in this other project why I implemented the fingerprint this way : astral-sh/ruff#7203
I also added this explanation in the PHPdoc of this method.

Comment on lines +72 to +74
$actual = json_encode($actual, \JSON_PRETTY_PRINT | \JSON_UNESCAPED_SLASHES);

static::assertSame($expected, $actual);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'm not sure how to fix the issue for the Window build.

Maybe you could compare the json instead with assertJsonStringEqualsJsonString ?

@VincentLanglet
Copy link
Copy Markdown
Owner

If you rebase, I fixed the phpstan build with #358

@AlexisPPLIN AlexisPPLIN force-pushed the add-gitlab-reporter branch from 9388c4b to 70270e5 Compare April 25, 2025 08:48
@AlexisPPLIN
Copy link
Copy Markdown
Contributor Author

Rebase on main done !

@VincentLanglet VincentLanglet merged commit 3f02722 into VincentLanglet:main Apr 27, 2025
16 of 17 checks passed
@VincentLanglet
Copy link
Copy Markdown
Owner

Hi @AlexisPPLIN, I reworked a little the reporter and just made a release ; please tell me if I broke something.

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.

2 participants