feat: GitHub actions reporter #1231
Conversation
commit: |
|
I think you should enable this reporter to knip's own test suite |
| }; | ||
| }; | ||
|
|
||
| const core = createGitHubActionsLogger(); |
There was a problem hiding this comment.
A bit of a nitpick I guess, but we need this only if the reporter's actually being used, so I think this could go into the reporter function.
There was a problem hiding this comment.
Totally fair, moved this now.
| `${cwd}/src/unused.ts | ||
| ::error file=${cwd}/src/index.ts::Unlisted dependencies | ||
| ::error file=${cwd}/src/index.ts::Unlisted dependencies | ||
| ::error file=${cwd}/src/index.ts,line=8,endLine=8,col=23,endColumn=23::Unresolved imports` |
There was a problem hiding this comment.
Would be great if there's coverage for warn(ing) too.
On a related not, would perhaps be good to find and use a fixture that has unused export and dependency for more diverse output (e.g. fixtures/rules or fixtures/workspaces or fixtures/plugins/nuxt, or multiple separate ones)
Btw I should add pos info to unlisted dependencies (WIP!)
There was a problem hiding this comment.
Added tests against the suggested fixtures.
There was a problem hiding this comment.
This GithubActions reporter should work in a way where when position info is added it will start using it without it needing to be updated. In order for it to use annotations in the files section the annotation has to have some sort of position info so by default I set it to just fall back to the beginning of the file. Without setting position info it will only show the annotation in the summary and not in the files tab.
There was a problem hiding this comment.
Thanks so much for picking this up, I have been totally swamped with other work!
I agree with @arthurfiorette that a good application for this would be Knip's own CI here in GitHub Actions!
@webpro Thoughts on this? Would you like the Knip repo to use this reporter in GHA? |
|
Dogfooding might be a good idea! We can add the Any chance we can see it in action somehow somewhere? |
Co-authored-by: Arthur Fiorette <[email protected]>
|
Here's an example repo using the github actions reporter |
|
Thanks a bunch, Cyle! Happy to merge. I've tested things and it LGTM. Any chance @EzraBrooks and @arthurfiorette can confirm this matches their expectations in their project(s) as well? |
|
This is a great first cut! It would be awesome to later make it annotate the package.json entries for unused dependencies, but I don't think that's necessary for the MVP. |
Great news is that unused dependencies was already accounted and tested for. I also now added an used dependency to my knip playground test repo now as well if you are interested in seeing that "live in living color". This actions reporter should also be fairly future tolerant. So that when the work to add pos info in unlisted deps lands it would work without needing changes to this reporter. |
|
What more can we ask for, Cyle? 🙏 Great job. |
closes #1012
Builds of the work from @arthurfiorette and @EzraBrooks to add a github actions reporter to knip.
Replaces
@actions/coredependency with a simple local logger.An example repo using the reporter can be found at https://github.com/cylewaitforit/knip-test/pull/1/files.