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

Make Resharper inspections fail the CI job #4514

Merged
merged 5 commits into from
Jun 16, 2021

Conversation

smoogipoo
Copy link
Contributor

@smoogipoo smoogipoo commented Jun 15, 2021

Fixes #4502

For the time being, I've published my own nuget package (smoogipoo.nvika) which has a GitHub build server built in. I've PR'd the change (laedit/vika#290) and intend to use the official package once that's merged in. And if it isn't merged, I'll adopt it into the ppy org.

It can be seen working here: https://github.com/smoogipoo/osu-framework/pull/28/checks

  • CI job fails
  • Annotations in the Files changed view.

@smoogipoo
Copy link
Contributor Author

I'm going to be force-pushing this one twice just for display purposes (draft PR). Ignore.

@smoogipoo smoogipoo marked this pull request as ready for review June 15, 2021 12:36
@bdach
Copy link
Collaborator

bdach commented Jun 15, 2021

So I was doing some recon on doing the same thing as this PR does, and I found this community answer that outlines yet another baffling actions limitation, which is why I haven't PR'd something like this yet:

Currently, GitHub Actions does not support so much annotations in a workflow run, the below is a summary of the limitation:

  • 10 warning annotations and 10 error annotations per step
  • 50 annotations per job (sum of annotations from all the steps)
  • 50 annotations per run (separate from the job annotations, these annotations aren’t created by users

We may get 50 instead of 10 if the nvika fork is adjusted to use API instead of those workflow commands (and it'll probably need to be in the other workflow to get tokens, same as the test report thing). But the best interim solution may be to upload the inspectcode XML report as a build artifact in the cases where 10/50 errors doesn't suffice...

@smoogipoo
Copy link
Contributor Author

Oh god why...

@bdach
Copy link
Collaborator

bdach commented Jun 15, 2021

I guess if we go down the API/separate workflow run route then we can do the same thing that the test plugin does and have a full summary in the check description even if we run out of the annotation limit (see here for example)...

@bdach
Copy link
Collaborator

bdach commented Jun 15, 2021

Also going to note that github have their own strongly-typed http client for their API at https://github.com/octokit/octokit.net. May be easier to use than raw requests if you decide on revisiting that path.

@smoogipoo
Copy link
Contributor Author

smoogipoo commented Jun 16, 2021

I think this can be merged as is in any case - at least it will fail CI even if it doesn't give the full scope of issues. Code inspection can always be run locally via build.sh or InspectCode.sh (after #4517).

I'll work on NVika again at some point, but want to change gears for the time being.

@peppy peppy merged commit 3c860da into ppy:master Jun 16, 2021
smoogipoo added a commit to smoogipoo/osu that referenced this pull request Jun 16, 2021
@smoogipoo smoogipoo deleted the fix-resharper-ci branch September 11, 2023 02:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GitHub actions CI doesn't output inspectcode errors
3 participants