-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ty] Link directly to typing conformance test suite when commenting the diff #19736
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
A small quality-of-life improvement to make it easier to evaluate whether a typing-conformance diff is good or bad
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
dhruvmanila
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
| echo '<!-- generated-comment typing_conformance_diagnostics_diff -->' >> comment.txt | ||
| echo '## Diagnostic diff on typing conformance tests' >> comment.txt | ||
| echo "## Diagnostic diff on [typing conformance tests](https://github.com/python/typing/tree/${CONFORMANCE_SUITE_COMMIT}/conformance)" >> comment.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: it might be worth linking directly to the conformance/tests directory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I considered it, but I like that this directory has a README.md file (which might be useful for first-time contributors!), whereas the tests subdirectory doesn't :-)
|
Ugh, it appears to be uploading the commit correctly: https://github.com/astral-sh/ruff/actions/runs/16730858154?pr=19669 And I think it is downloading it correctly: https://github.com/astral-sh/ruff/actions/runs/16730894339/job/47358456147 but the links are not making it into the comment correctly, for whatever reason: #19669 (comment) |
## Summary This PR fixes the issue mentioned in #19736 (comment) ~~but I can't test it without merging it on `main` because GitHub Actions still pickup the old version of the workflow file.~~ and is tested by manually triggering the workflow, refer to the comment on this PR (#19754 (comment)) which has the commit hash.
Summary
A small quality-of-life improvement to make it easier to evaluate whether a typing-conformance diff is good or bad
Test Plan