Skip to content

Conversation

@bcoe
Copy link
Contributor

@bcoebcoe commented Oct 25, 2020

Turns off coverage comments for the time being, until we can sort out #35759

Refs #35759, #35779, #35753

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@bcoebcoe requested review from Qard, Trott and watildeOctober 25, 2020 15:53
@bcoe
Copy link
ContributorAuthor

bcoe commented Oct 25, 2020

@thomasrockhu, we're still getting value out of the reports 👏 (for instance, see #35797 (comment)), but quite a few folks have been confused by reported drops in coverage, not related to their PRs.

An idea

Rather than providing information about drops in coverage as a comment, it would be useful to simply link to the coverage report for a sha in a comment, this would allow folks to see whether new tests they've added have actually increased coverage.

@nodejs-github-bot

This comment has been minimized.

@bcoebcoe added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fast-track PRs that do not need to wait for 48 hours to land. labels Oct 25, 2020
@bcoe
Copy link
ContributorAuthor

bcoe commented Oct 25, 2020

@Qard@watilde@Trott any objection to fast tracking this, I'd like to stop confusing folks until we sort out why the coverage reports jump around a bit.

👍 to fast track.

@nodejs-github-bot
Copy link
Collaborator

Turns off coverage comments for the time being, until we can sort out issues. PR-URL: nodejs#35800 Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Rich Trott <[email protected]>
@Trott
Copy link
Member

Landed in 4ace92f

@TrottTrott merged commit 4ace92f into nodejs:masterOct 26, 2020
targos pushed a commit that referenced this pull request Nov 3, 2020
Turns off coverage comments for the time being, until we can sort out issues. PR-URL: #35800 Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Rich Trott <[email protected]>
@targostargos mentioned this pull request Nov 3, 2020
BethGriggs pushed a commit that referenced this pull request Dec 8, 2020
Turns off coverage comments for the time being, until we can sort out issues. PR-URL: #35800 Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Rich Trott <[email protected]>
BethGriggs pushed a commit that referenced this pull request Dec 10, 2020
Turns off coverage comments for the time being, until we can sort out issues. PR-URL: #35800 Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Rich Trott <[email protected]>
@BethGriggsBethGriggs mentioned this pull request Dec 10, 2020
@targostargos added dont-land-on-v14.x and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Apr 30, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fast-trackPRs that do not need to wait for 48 hours to land.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@bcoe@nodejs-github-bot@Trott@Qard@watilde@targos