Skip to content

Conversation

bcoe
Copy link
Contributor

@bcoe bcoe 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

@bcoe bcoe requested review from Qard, Trott and watilde October 25, 2020 15:53
@bcoe
Copy link
Contributor Author

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.

@bcoe bcoe 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
Contributor Author

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 <daijiro.wachi@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@Trott Trott force-pushed the turn-off-coverage branch from 3452f61 to 4ace92f Compare October 26, 2020 10:58
@Trott
Copy link
Member

Trott commented Oct 26, 2020

Landed in 4ace92f

@Trott Trott merged commit 4ace92f into nodejs:master Oct 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 <daijiro.wachi@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@targos targos 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 <daijiro.wachi@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
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 <daijiro.wachi@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@BethGriggs BethGriggs mentioned this pull request Dec 10, 2020
@targos targos 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 free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fast-track PRs 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