X Tutup
The Wayback Machine - https://web.archive.org/web/20220423200511/https://github.com/nodejs/node/pull/35800
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: turn off coverage comments #35800

Merged
merged 1 commit into from Oct 26, 2020
Merged

test: turn off coverage comments #35800

merged 1 commit into from Oct 26, 2020

Conversation

Copy link
Member

@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 Trott, watilde and Qard Oct 25, 2020
@bcoe
Copy link
Member Author

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

Qard
Qard approved these changes Oct 25, 2020
@nodejs-github-bot

This comment has been minimized.

@bcoe bcoe added author ready fast-track labels Oct 25, 2020
@bcoe
Copy link
Member Author

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

@nodejs-github-bot nodejs-github-bot commented Oct 26, 2020

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
Copy link
Member

@Trott Trott commented Oct 26, 2020

Landed in 4ace92f

@Trott Trott merged commit 4ace92f into nodejs:master Oct 26, 2020
15 checks passed
targos pushed a commit that referenced this issue 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 issue 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 issue 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 labels Apr 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dont-land-on-v14.x fast-track
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants
X Tutup