FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: turn off coverage comments by bcoe · Pull Request #35800 · nodejs/node · GitHub

/ node Public

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 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 requested review from Qard, Trott and watilde October 25, 2020 15:53

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.

This comment has been minimized.

bcoe added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. fast-track PRs that do not need to wait for 48 hours to land. labels Oct 25, 2020

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.

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 force-pushed the turn-off-coverage branch from 3452f61 to 4ace92f Compare October 26, 2020 10:58

Trott commented Oct 26, 2020

Copy link
Copy Markdown
Member

Landed in 4ace92f

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 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 mentioned this pull request Dec 10, 2020
targos added dont-land-on-v14.x and removed author ready PRs that have at least one approval, no outstanding review comments, and a CI started. labels Apr 30, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL