RFC: Reviewdog Diagnostic Format #629

haya14busa · 2020-06-15T10:10:06Z

This is a proposal of ~~Reviewdog Diagnostic Protocol (RDP)~~ Reviewdog Diagnostic Format as standard
diagnostic format.
Any review, suggestion, feedback, criticism, and comments from anyone are very
much welcome.

Preview: https://github.com/reviewdog/reviewdog/tree/rdp/proto/rdf

Updated Unreleased section in CHANGELOG or it's not notable changes.

kayoub5 · 2020-06-15T12:03:37Z

proto/rdp/README.md

+https://github.com/reviewdog/reviewdog repository, but we may create a separate
+repository once it's reviewed and stabilized.
+
+# Reviewdog Diagnostic Protocol (RDP)


The abbreviation RDP will create ambiguity with the popular Remote Desktop Protocol.
I think it would be better to find another abbreviation.

True. Do you have any suggestions?
I wonder whether protocol is good or not as well. https://github.com/reviewdog/reviewdog/tree/rdp/proto/rdp#open-questions

But if we use Format instead of Protocol, RDF (Reviewdog Diagnostic Format) is also used by Resource Description Framework as well.

The acronym is basically for providing short format name (currently rdpjsonl / rdpjson), so maybe it's ok even if there are other usages. Any thought?

I'd rather not collide w/ RDF / RDP -- I use both.

The lazy answer would be RDDP or RDDF.

kayoub5 · 2020-06-15T12:21:05Z

proto/rdp/README.md

+        "range": {
+          "start": {
+            "line": 14,
+            "column": 15


Some compilers/linters can only provide absolute position in the file, not line/column.

Yeah, that's actually a good discussion point. The reason why I didn't include the byte offset (absolute position in the file) is that if we added it, all consumers tools of RDP output (such as reviewdog) need to add support to convert byte offset <-> line/column.

My current idea is the conversion should happen outside RDP, such as in linters or converters.

kayoub5 · 2020-06-15T12:23:50Z

@haya14busa Have you considered using an existing protocol? such as LSP?
https://microsoft.github.io/language-server-protocol/specifications/specification-current/#diagnostic

haya14busa · 2020-06-15T12:27:37Z

Have you considered using an existing protocol? such as LSP?

Considered, but no.
See https://github.com/reviewdog/reviewdog/tree/rdp/proto/rdp#language-server-protocol-lsp

kayoub5 · 2020-06-15T12:37:33Z

@haya14busa See also

kayoub5 · 2020-06-15T12:40:59Z

proto/rdp/reviewdog.proto

+// - rdpjsonl: JSON Lines (http://jsonlines.org/) of the `Diagnostic` message.
+// - rdpjson: JSON format of the `DiagnosticResult` message.
+
+syntax = "proto3";


I think using a JSON Schema file to describe the format would be easier for others to use and validate their output

I prefer Protocol Buffer (as primary IDL) because...

Protobuf can auto-generate code for a lot of languages including Go which is used in reviewdog.

It's more readable (I believe) and I think it's actually not difficult to understand. It supports comments for example.

But, yes that's a good point.
It should be able to auto-generate json schema from protobuf so we can give it a try. https://github.com/chrusty/protoc-gen-jsonschema

Added. 3e7cb51

haya14busa · 2020-06-15T13:11:11Z

I happened to find Static Analysis Results Interchange Format (SARIF) https://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html

haya14busa · 2020-06-15T18:58:37Z

I found SARIF 6 hours ago by finding @microsoft/eslint-formatter-sarif while investigating eslint custom formatter and I just found that GitHub's new Code Scanning (beta) feature uses SARIF for third-party code scanning support. https://help.github.com/en/github/finding-security-vulnerabilities-and-errors-in-your-code/about-code-scanning#about-third-party-code-scanning-tools (I got an email just 40 min ago!)

This is very interesting. My proposal can be still useful for some use cases like reviewdog or as a light weight format but discarding my proposal and adopting SARIF can be plausible alternative option.
I'll investigate it more and update the document.

jsoref

I don't have time right now, but I'd suggest copying the markdown file rendering into Grammarly and addressing most of its comments.

proto/rdp/README.md

jsoref · 2020-06-15T21:07:05Z

proto/rdp/README.md

+https://github.com/reviewdog/reviewdog repository, but we may create a separate
+repository once it's reviewed and stabilized.
+
+# Reviewdog Diagnostic Protocol (RDP)


I'd rather not collide w/ RDF / RDP -- I use both.

The lazy answer would be RDDP or RDDF.

proto/rdp/README.md

Co-authored-by: Josh Soref <jsoref@users.noreply.github.com>

jsoref · 2020-06-16T16:37:47Z

proto/rdp/README.md

+
+## Reviewdog Diagnostic Protocol Concept
+Again, the idea behind the Reviewdog Diagnostic Protocol (RDP) is to standardize
+the protocol for how diagnostic tools (e.g. compilers, linters, etc..) and


In addition to the language tool complaint, as a general rule you should not do e.g. ..., etc. -- e.g. is used to introduce examples, but etc. (et cetera) is for and others, which aren't really an example.

Here, the sample solution is to drop etc.. entirely. Possibly adding and or or before code review API.
Note: there's no need for the trailing . in this, as the parenthetical isn't a sentence.

haya14busa · 2020-07-21T14:42:10Z

Thank you everyone for the reviews so far!

I updated the document. Renamed to Reviewdog Diagnostic Format (RD format) and added a section about the SARIF.

I'm going to merge this PR because new upcoming features (like supporting suggestion) depends on it.
However, I still welcome any reviews and feedback.
Please feel free to leave any comments here or #628.

@haya14busa

https://github.com/reviewdog/reviewdog/tree/reviewdog:master # Changelog All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] ### ✨ Release Note  ### 🚀 Enhancements - [reviewdog#888](reviewdog#888) Allow GitHub PR reporting for a forked repository iff it's triggered by `pull_request_target` ### 🐛 Fixes - ... ### 🚨 Breaking changes - ... --- ## [v0.11.0] - 2020-10-25 ### ✨ Release Note reviewdog v0.11 introduced [Reviewdog Diagnostic Format (RDFormat)](./README.md#reviewdog-diagnostic-format-rdformat) as generic machine-readable diagnostic format and it unlocks new rich features like code suggestions. ### 🚀 Enhancements - [reviewdog#629](reviewdog#629) Introduced Reviewdog Diagnostic Format. - [reviewdog#674](reviewdog#674) [reviewdog#703](reviewdog#703) Support rdjsonl/rdjson as input format - [reviewdog#680](reviewdog#680) github-pr-review: Support multiline comments - [reviewdog#675](reviewdog#675) [reviewdog#698](reviewdog#698) github-pr-review: Support suggested changes - [reviewdog#699](reviewdog#699) Support diff input format (`-f=diff`). Useful for suggested changes. - [reviewdog#700](reviewdog#700) Support to show code(rule), code URL and severity in GitHub and GitLab reporters. - [reviewdog#678](reviewdog#678) github-pr-review: Support Code Suggestions - Introduced [reviewdog/action-suggester](https://github.com/reviewdog/action-suggester) action. - Introduced [reviewdog/action-setup](https://github.com/reviewdog/action-setup) GitHub Action which installs reviewdog easily including nightly release. - [reviewdog#769](reviewdog#769) Integration with [Bitbucket Code Insights](https://support.atlassian.com/bitbucket-cloud/docs/code-insights/) and [Bitbucket Pipelines](https://bitbucket.org/product/ru/features/pipelines) --- ## [v0.10.2] - 2020-08-04 ### 🐛 Fixes - [reviewdog#709](reviewdog#709) Check for GITHUB_ACTIONS instead of GITHUB_ACTION --- ## [v0.10.1] - 2020-06-30 ### 🚀 Enhancements - [reviewdog#563](reviewdog#563) Use `CI_API_V4_URL` environment variable when present. ### 🐛 Fixes - [reviewdog#609](reviewdog#609) reviewdog command will fail with unexpected tool's error for github-check/github-pr-check reporters as well. ([@haya14busa]) - [reviewdog#603](reviewdog#603) Fixed detection of Pull Requests from forked repo. ([@haya14busa]) --- ## [v0.10.0] - 2020-05-07 ### ✨ Release Note With v0.10.0 release, now reviewdog can find issues outside diff by controlling filtering behavior with `-filter-mode`. Also, you can ensure to check reported results by exit 1 with `-fail-on-error`. Example ```shell $ cd subdir/ && reviewdog -filter-mode=file -fail-on-error -reporter=github-pr-review ``` ### 🚀 Enhancements - [reviewdog#446](reviewdog#446) Added `-fail-on-error` flag ([document](https://github.com/reviewdog/reviewdog/tree/e359505275143ec85e9b114fc1ab4a4e91d04fb5#exit-codes)) and improved exit code handling. ([@DmitryLanda](https://github.com/DmitryLanda), [@haya14busa]) - [reviewdog#187](reviewdog#187) Added `-filter-mode` flag [`added`, `diff_context`, `file`, `nofilter`] ([document](https://github.com/reviewdog/reviewdog/tree/e359505275143ec85e9b114fc1ab4a4e91d04fb5#filter-mode)) which controls how reviewdog filter results. ([@Le6ow5k1](https://github.com/Le6ow5k1), [@haya14busa]) - [reviewdog#69](reviewdog#69) Support gerrit! ([@staticmukesh](https://github.com/staticmukesh)) - [reviewdog#548](reviewdog#548) Introduced nightly release ([reviewdog/nightly](https://github.com/reviewdog/nightly)). ([@haya14busa]) ### 🐛 Fixes - [reviewdog#461](reviewdog#461) All reporters now supports sub-directory run. ([@haya14busa]) ### 🚨 Breaking changes - `github-check` reporter won't report results outside diff by default now. You need to use `-filter-mode=nofilter` to keep the same bahavior. --- See https://github.com/reviewdog/reviewdog/releases for older release note. [Unreleased]: reviewdog/reviewdog@v0.10.0...HEAD [v0.10.0]: reviewdog/reviewdog@v0.9.17...v0.10.0 [v0.10.1]: reviewdog/reviewdog@v0.10.0...v0.10.1 [v0.10.2]: reviewdog/reviewdog@v0.10.1...v0.10.2 [v0.11.0]: reviewdog/reviewdog@v0.10.2...v0.11.0 [@haya14busa]: https://github.com/haya14busa

haya14busa force-pushed the rdp branch 3 times, most recently from 6e5adc2 to b2ea1e5 Compare June 15, 2020 10:17

RFC: Reviewdog Diagnostic Protocol (RDP)

8440424

haya14busa force-pushed the rdp branch from b2ea1e5 to 8440424 Compare June 15, 2020 10:20

haya14busa mentioned this pull request Jun 15, 2020

Reviewdog Diagnostic Format (RD Format, rdformat) #628

Open

20 tasks

Fix grammer errors reported by LanguageTool

7c1398d

haya14busa requested a review from a team June 15, 2020 10:35

haya14busa added 2 commits June 15, 2020 11:06

RDP is already used by Remote Desktop Protocol

bbf4b33

tweak RDP document

f1f0481

kayoub5 reviewed Jun 15, 2020

View reviewed changes

Add Json Schema

3e7cb51

jsoref reviewed Jun 15, 2020

View reviewed changes

haya14busa force-pushed the rdp branch from 6537441 to 3d84323 Compare June 16, 2020 14:47

Update proto/rdp/README.md

d7647fd

Co-authored-by: Josh Soref <jsoref@users.noreply.github.com>

haya14busa force-pushed the rdp branch from 3d84323 to d7647fd Compare June 16, 2020 14:53

fix nor

9e6518f

jsoref reviewed Jun 16, 2020

View reviewed changes

haya14busa mentioned this pull request Jul 1, 2020

Support SARIF as output format experimentally reviewdog/errorformat#66

Merged

haya14busa added 3 commits July 20, 2020 03:01

Merge branch 'master' into rdp

116d662

rename Reviewdog Diagnostic Protocol to Reviewdog Diagnostic Format

400ba97

fix English

ee26454

haya14busa force-pushed the rdp branch from 8600c30 to ee26454 Compare July 20, 2020 06:26

haya14busa added 2 commits July 20, 2020 15:13

More line range examples

832ae74

Update concept architecture image

c0c7733

haya14busa changed the title ~~RFC: Reviewdog Diagnostic Protocol (RDP)~~ RFC: Reviewdog Diagnostic Format Jul 21, 2020

haya14busa added 2 commits July 21, 2020 14:17

Update RD Format document: Add SARIF section

a75b840

Update CHANGELOG

5e689ab

haya14busa merged commit 0988ded into master Jul 21, 2020

haya14busa deleted the rdp branch July 21, 2020 14:50

Uh oh!

RFC: Reviewdog Diagnostic Format #629

RFC: Reviewdog Diagnostic Format #629

Uh oh!

Conversation

haya14busa commented Jun 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

kayoub5 commented Jun 15, 2020

Uh oh!

haya14busa commented Jun 15, 2020

Uh oh!

kayoub5 commented Jun 15, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haya14busa Jun 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haya14busa commented Jun 15, 2020

Uh oh!

haya14busa commented Jun 15, 2020

Uh oh!

jsoref left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

haya14busa commented Jul 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

haya14busa commented Jun 15, 2020 •

edited

Loading

haya14busa Jun 15, 2020 •

edited

Loading

haya14busa commented Jul 21, 2020 •

edited

Loading