search

cpp-linter / cpp-linter-action

A Github Action for linting C/C++ code integrating clang-tidy and clang-format to collect feedback provided in the form of file-annotations, thread-comments, workflow step-summary, and Pull Request reviews.
https://cpp-linter.github.io/cpp-linter-action/
MIT License
91 stars 21 forks source link

Enable parallelism #213

Closed jnooree closed 6 months ago

jnooree commented 6 months ago

See cpp-linter/cpp-linter#92 for the related CLI updates.

2bndy5 commented 6 months ago

FYI, dependabot will submit a PR that will update the CLI pkg when we release the changes in cpp-linter/cpp-linter#92. Until then we have other testing that we're trying to get done, so this will be in idle for the meantime.

shenxianpeng commented 6 months ago

@2bndy5 I think we should publish a patch release of the current draft release (backlog). Later after this PR and cpp-linter version bump to v1.8.0, we should publish a minor release. so split release can ensure stability and ease of control. your throughs?

2bndy5 commented 6 months ago

Yeah, we can do a patch release right now. I have no objection to that.

As for parallelism, it should definitely be a minor version bump. I just did the same bump for cpp-linter.

Once dependabot submits the PR for cpp-linter v1.8.0, I'll be rebasing this branch and supplementing it with doc info.

2bndy5 commented 6 months ago

Ok, I force-pushed a docs-related commit after rebasing the branch.

I think the self-test CI failed again because the PR is coming from a public fork, thus the token permission is inadequate to post a thread comment. This is what the pull_request_target event is supposed to be for, but I don't understand how it would help test the proposed changes.

ERROR:CPP Linter:response returned 403 from POST https://api.github.com/repos/cpp-linter/cpp-linter-action/issues/213/comments with message: {"message":"Resource not accessible by integration","documentation_url":"https://docs.github.com/rest/issues/comments#create-an-issue-comment"}

This message is not entirely informative, but that's all we get from github REST API 403 response.

shenxianpeng commented 6 months ago

Introduce pull_request_target should work. the current failure of post comments shouldn't affect anything.

2bndy5 commented 6 months ago

the current failure of post comments shouldn't affect anything

Agreed. I don't see a good workaround right now. I added the pull_request_target trigger, but it won't take affect til merged to main.

shenxianpeng commented 6 months ago

pull_request_target and pull_request are more confusing now. Btw, self test run success after the merge.

2bndy5 commented 6 months ago

It is designed to let a "known good" CI workflow (the target branch's config) run changes in the PR. But, it ignores any incoming changes in the CI workflows config.

There is no way to allow comments to be posted using PR changes in a CI workflow from a public fork. That is the exact security risk that pull_request_target is designed to avoid.