Stop duplicate code comments for code review.

[entvex]

Description

When reviewing a pull request, duplicated errors pollute the code view unnecessarily.

Example

When reviewing code. We just want a clean view to do it.

[tisonkun]

We may still want to verify the docs syntax are valid?

[entvex]

We may still want to verify the docs syntax are valid?

Yes, I understand the intention. But there must be another way to do it, so the code reviewers don't have to see all these duplicates.

Another approach could be if we spun this code into a separate GHA that also triggers on push, like the action it is in now. But do you know if that would stop the duplicates, or would they still show up in the review?

Or we could verify the syntax when creating a release (the new GHA, which I am working on).

[entvex]

Today I have tried to do as I suggested and spun the docfx code away from the ci-unit.yaml and into another GHA named doc-syntax-check.yaml. But it sadly didn't fix the issue, and therefore the duplicated items are still an issue.

I still think we should remove the docfx from CI, so we don't see duplicates when reviewing code.

[tisonkun]

OK.

The reason seems to be that dotnet build in the doc-tests generate annotations which should not be forcely applied. If we can disable the annotation for doc-tests suspected actions/setup-dotnet introduced it, we can keep the test (It's unreasonable we disable a test because it generates extra warning instead of fix it).

But given that the docs should be relatively stably correct and we can fix any violations later spotted, we can disable it first.

[tisonkun]

Replace dotnet build -p:TargetFramework=net7.0 with dotnet build -p:TargetFramework=net7.0 --property WarningLevel=0 should help.

Functionality checks are covered by unit-tests, we run it in doc-tests for prepare infos only.

[entvex]

Replace dotnet build -p:TargetFramework=net7.0 with dotnet build -p:TargetFramework=net7.0 --property WarningLevel=0 should help.

Functionality checks are covered by unit-tests, we run it in doc-tests for prepare infos only.

Okay. So I tried to use the code in your comment, here in my test repo https://github.com/entvex/pulsar-dotpulsar/pull/2/files. And it looks like the normal unit test will make errors if there are any doc XML syntax errors.

Therefor, I really think we only need to run doc test, when we do a release. And therefore it does not need to be part of the CI unit test pipeline.

Does it make sense ?

[tisonkun]

Although the docfx docs/api/docfx.json should run more test than dotnet build and a per PR test should prevent us to postpone all potential issues until releases, I don't insist to block this patch and approve its merge.