feat: provide Codeclimate compatible report fromat #367

tploss · 2024-09-05T07:58:42Z

Provide code quality report as JSON according to Code Climate specificiation. This is need when using the editorconfig-checker in GitLab CI.

More details: #365

codecov · 2024-09-05T09:00:34Z

Codecov Report

Attention: Patch coverage is 89.65517% with 3 lines in your changes missing coverage. Please review.

Project coverage is 87.10%. Comparing base (d6c17be) to head (ca2018a).
Report is 40 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/error/error.go	78.57%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #367      +/-   ##
==========================================
+ Coverage   84.89%   87.10%   +2.21%     
==========================================
  Files           8        9       +1     
  Lines         695      636      -59     
==========================================
- Hits          590      554      -36     
+ Misses         81       56      -25     
- Partials       24       26       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

klaernie

Looks generally good and like it adheres to the specification you left in the comment. Thanks for turning that ugly if...elseif..else into a switch - I totally forgot to think about if that feature is available in golang when writing it.. ;)

However I'd like to see tests for demonstrating it in action and proving correctness.

Furthermore I think you should disable the verbose and debug logging when config.Format == codeclimate - else anyone trying to debug will surround the JSON output with unparsable noise.

Also I thought about, if one should maybe turn the codeclimate generation into a method of ValidationErrors, which only runs ConsolidateErrors and returns the JSON.

In that regard, I was wondering lately, if it would be better to split FormatErrors into per-output-format functions, and change their interface towards main() to utilize an io.Writer. But that could easily be another PR once this one is merged.

pkg/error/format_codeclimate.go

tploss · 2024-09-06T05:32:24Z

Looks generally good and like it adheres to the specification you left in the comment. Thanks for turning that ugly if...elseif..else into a switch - I totally forgot to think about if that feature is available in golang when writing it.. ;)

However I'd like to see tests for demonstrating it in action and proving correctness.

👍
Will definitely create some tests. Just have to first understand this snapshot system for test data.

Furthermore I think you should disable the verbose and debug logging when config.Format == codeclimate - else anyone trying to debug will surround the JSON output with unparsable noise.

An easier solution from my perspective would be to just log non-standard messages (so debug, error, etc) on stderr instead of stdout. Then one can easily just use editorconfig-checker --format codeclimate > report.json but still see all error and debug messages.
Thoughts?

Also I thought about, if one should maybe turn the codeclimate generation into a method of ValidationErrors, which only runs ConsolidateErrors and returns the JSON.

In that regard, I was wondering lately, if it would be better to split FormatErrors into per-output-format functions, and change their interface towards main() to utilize an io.Writer. But that could easily be another PR once this one is merged.

I was also not really happy with how a single function would grow per added format but did not want to refactor that completely in my PR that just wants to add a single format.
Can we do that as a separate PR afterwards?

klaernie · 2024-09-06T06:30:27Z

Will definitely create some tests. Just have to first understand this snapshot system for test data.

It's actually fairly easy to use: you just demonstrate calling your code in error_test.go and hand the result of calling FormatErrors to s.MatchSnapshot. Then run UPDATE_SNAPS=true go test ./... so it generates the new snapshot.

tploss · 2024-09-06T09:57:43Z

It's actually fairly easy to use: you just demonstrate calling your code in error_test.go and hand the result of calling FormatErrors to s.MatchSnapshot. Then run UPDATE_SNAPS=true go test ./... so it generates the new snapshot.

Done 🙂
I also verified the test by changing the severity constant to "major" and this subsequently failed the test case.

tploss · 2024-09-06T10:02:02Z

The ci is waiting for an approval before running? https://github.com/editorconfig-checker/editorconfig-checker/actions/runs/10736321284
I'm not really used to GitHub CI so I don't know if this is normal. @klaernie @mstruebing

klaernie · 2024-09-06T15:34:07Z

Indeed, the CI is waiting to be approved by a maintainer of the repo (so sadly I cannot trigger it).

klaernie · 2024-09-06T16:01:29Z

Oh, just noticed you overlooked the same places in the README and cmd/main.go argument help as me, which show the available output formats.

mstruebing · 2024-09-06T17:08: 8000 26Z

CI already ran so someone needs to have pressed the button.
It's a mechanism to make sure someone who does a PR doesn't break anything or leak any secrets etc.
You could do all kind of bad stuff with Github Actions, this makes sure that at least someone needs to approve the run.

Provide code quality report as JSON according to Code Climate specificiation. This is need when using the editorconfig-checker in GitLab CI. editorconfig-checker#365 editorconfig-checker#365

tploss · 2024-09-06T19:23:58Z

Oh, just noticed you overlooked the same places in the README and cmd/main.go argument help as me, which show the available output formats.

Thanks, I have fixed it and also provided a nicely formatted example output 🙂

theoludwig

Thank you!
LGTM!

ccoVeille · 2024-09-07T04:00:15Z

cmd/editorconfig-checker/main.go

@@ -127,7 +127,9 @@ func main() {
 	errorCount := len(formattedErrors)
 	if errorCount != 0 {
 		config.Logger.PrintLogMessages(formattedErrors)
-		config.Logger.Error(fmt.Sprintf("\n%d errors found", errorCount))
+		if config.Format != "codeclimate" {


I would add a comment to explain why you disable the log

ccoVeille · 2024-09-07T04:01:10Z

cmd/editorconfig-checker/main.go

@@ -127,7 +127,9 @@ func main() {
 	errorCount := len(formattedErrors)
 	if errorCount != 0 {
 		config.Logger.PrintLogMessages(formattedErrors)
-		config.Logger.Error(fmt.Sprintf("\n%d errors found", errorCount))
+		if config.Format != "codeclimate" {


I would like to suggest adding a constant for code climate

This way code that relies on this value could be easily identified

I would take it on me to split FormatErrors into multiple independent functions once this PR is merged. Now with a fourth format supported this evergrowing function is too hard to read.

I also wondered if there was a neat way to represent the collection of possible output formats, so that the argument parsing code can discover the possible formats without us having to every time adjust it by hand.

I would love that

It would help to have a type and a method to list all the formats.

The fact this code has to be maintained is q bit a pain

flag.StringVar(&c.Format, "format", "default", "specify the output format: default, gcc, github-actions, codeclimate")

Opened #374 to remind me of the refactoring.

Reading up on golang, I somehow did not find any mention how to implement essentially an enum type. @ccoVeille do you have seen anything alike in the past?

theoludwig · 2024-09-08T12:42:09Z

I've merged this PR, as it seems like it was ready to go. 😄

Feel free to open follow up PRs for refactoring.

…/cmd/editorconfig-checker to v3.1.1 (forgejo) (#6520) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [github.com/editorconfig-checker/editorconfig-checker/v3/cmd/editorconfig-checker](https://github.com/editorconfig-checker/editorconfig-checker) | minor | `v3.0.3` -> `v3.1.1` | --- ### Release Notes <details> <summary>editorconfig-checker/editorconfig-checker (github.com/editorconfig-checker/editorconfig-checker/v3/cmd/editorconfig-checker)</summary> ### [`v3.1.1`](https://github.com/editorconfig-checker/editorconfig-checker/releases/tag/v3.1.1) [Compare Source](editorconfig-checker/editorconfig-checker@v3.1.0...v3.1.1) ##### Bug Fixes - dockerfile expected binary at /, not /usr/bin/ [#410](editorconfig-checker/editorconfig-checker#410) ([#411](editorconfig-checker/editorconfig-checker#411)) ([2c82197](editorconfig-checker/editorconfig-checker@2c82197)) ### [`v3.1.0`](https://github.com/editorconfig-checker/editorconfig-checker/releases/tag/v3.1.0) [Compare Source](editorconfig-checker/editorconfig-checker@v3.0.3...v3.1.0) ##### Features - add zip version when compressing all binaries ([#321](editorconfig-checker/editorconfig-checker#321)) ([#362](editorconfig-checker/editorconfig-checker#362)) ([f1bb625](editorconfig-checker/editorconfig-checker@f1bb625)) - consolidate adjacent error messages ([#360](editorconfig-checker/editorconfig-checker#360)) ([cf4ae1c](editorconfig-checker/editorconfig-checker@cf4ae1c)) - editorconfig-checker-disable-next-line ([#363](editorconfig-checker/editorconfig-checker#363)) ([6116ec6](editorconfig-checker/editorconfig-checker@6116ec6)) - provide Codeclimate compatible report fromat ([#367](editorconfig-checker/editorconfig-checker#367)) ([282c315](editorconfig-checker/editorconfig-checker@282c315)) - support `.editorconfig-checker.json` config ([#375](editorconfig-checker/editorconfig-checker#375)) ([cb0039c](editorconfig-checker/editorconfig-checker@cb0039c)) ##### Bug Fixes - actually use the correct end marker ([#405](editorconfig-checker/editorconfig-checker#405)) ([3c03499](editorconfig-checker/editorconfig-checker@3c03499)) - add `.ecrc` deprecation warning ([#389](editorconfig-checker/editorconfig-checker#389)) ([d33b81c](editorconfig-checker/editorconfig-checker@d33b81c)) - this release-please marker ([#403](editorconfig-checker/editorconfig-checker#403)) ([617c6d4](editorconfig-checker/editorconfig-checker@617c6d4)) - typo in config, `SpacesAftertabs` => `SpacesAfterTabs` ([#386](editorconfig-checker/editorconfig-checker#386)) ([25e3542](editorconfig-checker/editorconfig-checker@25e3542)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - "* 0-3 * * *" (UTC), Automerge - "* 0-3 * * *" (UTC). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6520 Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org> Co-authored-by: Renovate Bot <forgejo-renovate-action@forgejo.org> Co-committed-by: Renovate Bot <forgejo-renovate-action@forgejo.org>

klaernie requested changes Sep 5, 2024

View reviewed changes

pkg/error/format_codeclimate.go Outdated Show resolved Hide resolved

tploss force-pushed the 365-codeclimate-format branch from 1c5011d to b6d0ce5 Compare September 6, 2024 09:54

klaernie approved these changes Sep 6, 2024

View reviewed changes

feat: provide Codeclimate compatible report fromat

dee883a

Provide code quality report as JSON according to Code Climate specificiation. This is need when using the editorconfig-checker in GitLab CI. editorconfig-checker#365 editorconfig-checker#365

tploss force-pushed the 365-codeclimate-format branch from b6d0ce5 to dee883a Compare September 6, 2024 19:22

theoludwig approved these changes Sep 6, 2024

View reviewed changes

theoludwig added this to the v3.1.0 milestone Sep 6, 2024

ccoVeille approved these changes Sep 7, 2024

View reviewed changes

klaernie mentioned this pull request Sep 7, 2024

Refactor error.go FormatErrors #374

Closed

theoludwig mentioned this pull request Sep 8, 2024

feat: support .editorconfig-checker.json config #375

Merged

theoludwig enabled auto-merge (squash) September 8, 2024 12:40

Merge branch 'main' into 365-codeclimate-format

ca2018a

theoludwig merged commit 282c315 into editorconfig-checker:main Sep 8, 2024
3 checks passed

klaernie mentioned this pull request Sep 8, 2024

Provide code quality report as JSON according to Code Climate specificiation #365

Closed

theoludwig linked an issue Sep 8, 2024 that may be closed by this pull request

Provide code quality report as JSON according to Code Climate specificiation #365

Closed

This was referenced Jan 5, 2025

chore(main): release 3.1.0 #402

Closed

chore(main): release 3.1.0 #404

Closed

editorconfig-checker-bot mentioned this pull request Jan 6, 2025

chore(main): release 3.1.0 #408

Merged

BrewTestBot mentioned this pull request Jan 8, 2025

editorconfig-checker 3.1.0 Homebrew/homebrew-core#203568

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: provide Codeclimate compatible report fromat #367

feat: provide Codeclimate compatible report fromat #367

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

feat: provide Codeclimate compatible report fromat #367

feat: provide Codeclimate compatible report fromat #367

Uh oh!

Conversation

Uh oh!

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!