Github semi-recently introduced a new feature to Action workflows that allows you to install little regex handlers for logs that create pull request review comments for matches. These are called problem matchers and are great for prominently surfacing things like linter failures, warnings and other issues that won’t necessarily fail the build. They seem to be a port of an existing feature in VSCode extensions: https://code.visualstudio.com/docs/editor/tasks#_processing-task-output-with-problem-matchers
I’m particularly excited about the compiler warnings matchers since they seem to sneak in and then every once in a while someone makes a pull request to remove them. People that develop on Linux don’t tend to check the Windows warnings and vica-versa, and as long as the CI results are green no one really digs into the logs. This would allow them to be caught early in the review process.
@csabella suggested I post about this to gauge interest and make it more discover-able. I’ve created three pull requests with problem matchers for:
Catches errors and warnings from sphinx documentation builds. As a bonus this allows for a workflow whereby people making and reviewing documentation pull requests can fully work entirely from Github. Between this problem matcher and the netlify doc preview, we can be fairly sure the changes are good to go. (This one is not as important since https://github.com/python/cpython/commit/6aabb63d96845b3cb207d28d40bf0b78e171be75 which adds -W, warnings-as-errors to the sphinx build but still provides errors in a more convenient place).
This allows them to be used by the wider Github community and hopefully have a wider set of eyes looking at them to catch any issues.
Despite the error formats being relatively stable, relying on me to maintain the actions might be a little queasy for some of the core developers. In this case, it’s possible to commit the matchers (here’s an example of one) under the .github folder on our repo and just use the local versions, I’d be happy to change over to that method if people feel necessary.
I think this sounds good. If it’s really just the matcher .json files that would need to be added to the .github folder, I think I would opt for that just for discoverability. I’m not concerned about maintenance of these matchers; they appear simple enough to likely not need much if any for quite some time.
However, if these warnings are going to start appearing on every pull request, we likely want to fix the any existing warnings and enable any warnings-as-errors options on the CI whose output these checks parse when we add them since otherwise people are likely to be confused about why their typo fix in a docstring is causing build warnings
I think this would be a great improvement. I have fixed many Windows and Linux compiler warnings in the past. I agree with @zware in that I think I would prefer to have these JSON files living in the repo.
@ammaraskar Could you make a PR adding the relevant files so we know how many warnings we have currently? Feel free to add me as a reviewer to such pr
Would you want to check the difference between master’s warnings and the current PR’s warnings and only put the new warnings in the PR’s comment?
An implementation is to commit (all of) the warnings as a text file with every PR, then the current PR can run diff on that file to get the text to display in the comment
The only warnings are from the MSVC build, which is what I anticipated since most core-devs exclusively build on Linux. You can see all the warnings for the MSVC build on the bottom, Unchanged files with check annotations part of https://github.com/python/cpython/pull/18532/files
