Support ignoring failures on comments outside PR diff range
- Dominant language
- TypeScript
- Stars
- 5.5k
- Forks
- 381
- Avg merge
- 2h 8m
- Merged PRs (30d)
- 6
Description
Hi there, Danger.JS is awesome and we use it extensively in our CI builds. Just ran into an issue where we're not getting updated comments/results on our Github PRs because of a plugin (a fork of one we use) that tries to translate eslint warnings/errors into Danger comments: https://github.com/sgtcoolguy/danger-plugin-eslint/blob/master/src/index.ts
Basically, it's possible that a user touches a file that has existing warnings from eslint, but doesn't edit the lines mentioned in the warning. The plugin will attempt to set the warning comment because the file is in the listing for the PR, but Github's API will throw an error that the "position" is invalid.
Maybe there's some way to have the plugin denote that the comment isn't "required" to be set or something? Or is there some way I could tell if the warning would have an invalid position by pre-filtering based on the diff chunks for the file? Then I could have it not do inline comments but just generally list the warning/error.
It's odd, given that it seems Danger was able to find a position here: https://github.com/danger/danger-js/blob/master/source/platforms/github/comms/issueCommenter.ts#L14 since the error came from the Github API response.
Relates to #770
Contributor guide
Research direction
The relevant entry point mentioned is source/platforms/github/comms/issueCommenter.ts; start there and inspect how GitHub positions are derived for warnings outside the PR diff. Reproduce the invalid-position response with the eslint plugin scenario, then verify that an out-of-range comment failure is ignored or surfaced as a non-inline result without preventing updated PR comments.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- github, typescript
- Domain
- api, devtools
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100