Skip to content

test: turn off coverage comments - #35800

Merged
Trott merged 1 commit into
nodejs:masterfrom
bcoe:turn-off-coverage
Oct 26, 2020
Merged

test: turn off coverage comments#35800
Trott merged 1 commit into
nodejs:masterfrom
bcoe:turn-off-coverage

Conversation

@bcoe

@bcoe bcoe commented Oct 25, 2020

Copy link
Copy Markdown
Contributor

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

bcoe commented Oct 25, 2020

Copy link
Copy Markdown
Contributor Author

@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 with CI started, the required approvals, and no outstanding review comments. fast-track PRs proposed for a shorter-than-standard waiting period before landing. labels Oct 25, 2020
@bcoe

bcoe commented Oct 25, 2020

Copy link
Copy Markdown
Contributor Author

@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
Copy Markdown
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

Trott commented Oct 26, 2020

Copy link
Copy Markdown
Member

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 with CI started, the required approvals, and no outstanding review comments. 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 proposed for a shorter-than-standard waiting period before landing.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants