ci: integration with TiCS code quality analysis #138
Conversation
|
@letFunny are the annotations in https://github.com/cjdcordeiro/chisel/actions/runs/9480198560 legit? The coverage issue we saw a few days ago has been fixed (the tool requires a pre-existing coverage report). |
cjdcordeiro
force-pushed
the
ROCKS-1209/ci-integration-with-tics
branch
from
11af61d to
5789563
Compare
cjdcordeiro
force-pushed
the
ROCKS-1209/ci-integration-with-tics
branch
from
5789563 to
d2c29fd
Compare
| ref: ${{ needs.set-project.outputs.ref }} | ||
| repository: ${{ needs.set-project.outputs.repo }} |
There was a problem hiding this comment.
Why can't the previous job be inlined here?
There was a problem hiding this comment.
technically it could, but there are a few reasons that led me to split it this way:
- segregation of concerns: the 1st one is a preparatory step, while the 2nd one is the analysis
- permissions: the 2nd one has additional permissions ("write to PRs") that the 1st one doesn't need
If there's a strong reason to merge the two, I'm not opposed to doing it.
| codetype: 'PRODUCTION' | ||
| project: ${{ env.TICSPROJECT }} | ||
| branchdir: . | ||
| filelist: ${{ env.TICS_FILELIST }} |
There was a problem hiding this comment.
We need to exclude testutil and the new apachetestutil maybe we should do it here directly.
There was a problem hiding this comment.
Noted. As per our discussion, testutil is indeed just used within the project's tests. And even though TiCS knows what's PRODUCTION vs TEST code, it will take those utilities as PRODUCTION code.
This PR is getting blocked for the time being, but I'll keep this thread open for future reference
There was a problem hiding this comment.
In the example I see a green checkmark with zero details about what it means to pass/not-pass, let alone the gradient that was accounted for. That's some very prominent space, with no clear benefits.
I may be missing something, but in principle if we are to integrate something like this, it should have better statistics, and ideally be integrated into the workflow like every other test runner we have.
|
Important This PR was discussed offline, and as per the above comment we've agreed that the feedback provided by this new CI action adds unnecessary noise to the PRs and is not very handy or relevant for assessing the PRs' quality. We're then Closing this PR and escalating the need for better integration of the results with the PRs. |
This PR introduces the TiCS analyzer with the Chisel CI.
How it works
On every PR
TiCS job will run in "client" mode, analyzing the changed files in the PR. As a result, it will create code annotations and write a comment to the PR with a summary of the analysis. Example: cjdcordeiro#12 (comment)
NOTE: this new workflow runs on
pull_request_targetsince the TiCS action requires a TICSAUTHTOKEN (secret). This means that we won't see this workflow being executed and annotated within this PR (#138) because it always runs within the context of the base (i.e.main), but since this is the first PR introducing this new workflow, it obviously doesn't yet exist inmainand thus won't be run here.On-demand and on-schedule
At least once a day, this job will also run in "qserver" mode, meaning that it will do a full analysis of the Chisel project and send the results to our remote dashboard in Tiobe. Here's an example of such run: https://github.com/cjdcordeiro/chisel/actions/runs/12831240744 (and the corresponding view in the dashboard)