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.

Sign up for GitHub

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

Jump to bottom

PR diffs are showing pages of unrelated warnings about "check annotations" #31347

Closed
sam-github opened this issue Jan 13, 2020 · 4 comments · Fixed by #31349
Closed

PR diffs are showing pages of unrelated warnings about "check annotations" #31347

sam-github opened this issue Jan 13, 2020 · 4 comments · Fixed by #31349
Labels
build Issues and PRs related to build files or the CI.

Comments

@sam-github
Copy link
Contributor

  • Version: ???
  • Platform: ???
  • Subsystem: ???

Random example from https://github.com/nodejs/node/pull/31337/files: pr31337.pdf

Its not clear what is causing this, but its quite distracting.

Does anyone know? Is it because of github actions?
@richardlau
Copy link
Member

These started appearing after we enabled builds using GitHub Actions workflows in #31153.

I attempted to get rid of them in #31311 but it looks like it only shows the first 10 failing checks so the ones I fixed are gone but it's now showing the next 10.

@richardlau
Copy link
Member

I think I've figured out what's going on... it just so happens that the warning messages coming out of the Visual Studio C/C++ compiler happen to match the regular expression in the tsc problem matcher installed by the setup-node action we use to run npx envinfo prior to the build.

node/.github/workflows/CI.yml

Lines 22 to 27 in f4a4a1a

- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v1
with:
node-version: 13.x
- name: Environment Information
run: npx envinfo

I don't see a way to turn off this problem matcher. Not using setup-node would prevent the problem matcher being installed.

cc @gengjiawen

@sam-github
Copy link
Contributor Author

I don't think the annotations are helpful to collaborators, and I think they may be actively discouraging and confusing to new contributors. Its not an emergency, but if they could be disabled, I think it would be an improvement to the contribution experience.

@richardlau richardlau mentioned this issue Jan 14, 2020
Merged
2 tasks
@richardlau
Copy link
Member

GitHub hosted runners already have a version of Node.js installed so we don't need to use setup-node to install another one to run npx envinfo. PR to remove it: #31349

@richardlau richardlau added the build Issues and PRs related to build files or the CI. label Jan 14, 2020
codebytere pushed a commit that referenced this issue Feb 17, 2020
@richardlau @codebytere
build: do not use setup-node in build workflows
2e216ae 
The setup-node GitHub Action installs problem matchers that happen
to match the warning message format of Visual Studio's C/C++ compiler.
This is resulting in all of our pull requests being annotated with
`Unchanged files with check annotations` which are confusing to new
contributors as they are not due to the changes in the pull request.

The action is used to run `npx envinfo` to dump some information into
the logs before the actual build. All GitHub hosted runners already
have a version of Node.js installed (12.x at the time of this commit)
which we can use to run `envinfo`. Remove the action to avoid using
the problematic problem matcher.

PR-URL: #31349
Fixes: #31347
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
codebytere pushed a commit that referenced this issue Mar 14, 2020
@richardlau @codebytere
build: do not use setup-node in build workflows
37e386f 
The setup-node GitHub Action installs problem matchers that happen
to match the warning message format of Visual Studio's C/C++ compiler.
This is resulting in all of our pull requests being annotated with
`Unchanged files with check annotations` which are confusing to new
contributors as they are not due to the changes in the pull request.

The action is used to run `npx envinfo` to dump some information into
the logs before the actual build. All GitHub hosted runners already
have a version of Node.js installed (12.x at the time of this commit)
which we can use to run `envinfo`. Remove the action to avoid using
the problematic problem matcher.

PR-URL: #31349
Fixes: #31347
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
MylesBorins pushed a commit to MylesBorins/node that referenced this issue Apr 2, 2020
@richardlau @MylesBorins
build: do not use setup-node in build workflows
9a3e059 
The setup-node GitHub Action installs problem matchers that happen
to match the warning message format of Visual Studio's C/C++ compiler.
This is resulting in all of our pull requests being annotated with
`Unchanged files with check annotations` which are confusing to new
contributors as they are not due to the changes in the pull request.

The action is used to run `npx envinfo` to dump some information into
the logs before the actual build. All GitHub hosted runners already
have a version of Node.js installed (12.x at the time of this commit)
which we can use to run `envinfo`. Remove the action to avoid using
the problematic problem matcher.

PR-URL: nodejs#31349
Fixes: nodejs#31347
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
MylesBorins pushed a commit that referenced this issue Apr 2, 2020
@richardlau @MylesBorins
build: do not use setup-node in build workflows
46f83df 
The setup-node GitHub Action installs problem matchers that happen
to match the warning message format of Visual Studio's C/C++ compiler.
This is resulting in all of our pull requests being annotated with
`Unchanged files with check annotations` which are confusing to new
contributors as they are not due to the changes in the pull request.

The action is used to run `npx envinfo` to dump some information into
the logs before the actual build. All GitHub hosted runners already
have a version of Node.js installed (12.x at the time of this commit)
which we can use to run `envinfo`. Remove the action to avoid using
the problematic problem matcher.

Backport-PR-URL: #32608
PR-URL: #31349
Fixes: #31347
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

build: do not use setup-node in build workflows richardlau/node-1
2 participants
@sam-github @richardlau