danger / danger-js

⚠️ Stop saying "you forgot to …" in code review
http://danger.systems/js/
MIT License
5.22k stars 363 forks source link

[BUG] Danger posting separate comment for each warning on GitHub #1413

Closed renspoesse closed 8 months ago

renspoesse commented 8 months ago

Describe the bug When calling warn() with the file and line parameters, Danger posts an inline PR comment for each violation, even though the reference documentation suggests that warn() outputs warnings in a single HTML table.

I'd like to get all warnings displayed in a single comment, where each warning still links to the file and line number it affects.

To Reproduce Steps to reproduce the behavior:

  1. warn("Something's not right", "src/TEST.md", 5);
  2. warn("Something's not right", "src/TEST.md", 6);
  3. warn("Something's not right", "src/TEST.md", 7);

This generates 3 comments on the GitHub PR.

Expected behavior

According to https://danger.systems/js/reference, it should show a single HTML table. Something like this (taken from the Ruby version of Danger we use in another repository):

4 Warnings
:warning: path-to-file.swift#L211 - 'CGFloat.contentWidthFactor' is deprecated. Use the '.contentWidthFactor()' method on your view controller.
:warning: path-to-file.swift#L212 - 'CGFloat.contentWidthFactor' is deprecated. Use the '.contentWidthFactor()' method on your view controller.
:warning: path-to-file.swift#L213 - 'CGFloat.contentWidthFactor' is deprecated. Use the '.contentWidthFactor()' method on your view controller.
:warning: path-to-file.swift#L214 - 'CGFloat.contentWidthFactor' is deprecated. Use the '.contentWidthFactor()' method on your view controller.

Generated by :no_entry_sign: Danger

If not generating a table is the expected behavior, what steps could I take to do generate a table?

Your Environment

software version
danger.js 11.3.0
node 18.16.0
npm 9.5.1
Operating System macOS 14.1.1
orta commented 8 months ago

I'd bake the link into your messages and not use the file + line syntax, I don't think there's an argument for disabling the inline commenting built into the sustem

fbartho commented 8 months ago

The inline commenting was added later so that might explain why these docs are a bit out-of-date, or at least don't mention that case.

Additionally, if the file/line is not present in a PR, then it will bundle those messages into a table.

Just for my reasoning on why this works this way: For many projects, an inline comment is much more actionable. You can even provide a triple-backtick suggestion and use GitHub's browser interface to commit a change.

Anyhow: I agree with @orta re: resolution of no behavior change!