-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add full output format changes to the changelog
#19968
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
Summary -- I thought this might warrant a small blog-style writeup, especially since we already got a question about it (#19966), but I'm happy to switch back to a one-liner under `### Other changes` if preferred. I'll copy whatever we add here to the release notes too. Do we need a note at the top about the late addition?
CHANGELOG.md
Outdated
|
|
||
| ## 0.12.9 | ||
|
|
||
| ### New default diagnostic format |
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 think this headline is a bit confusing because it suggests that we now use an entirely different output format, and that users have the option to switch back to the old format. However, this is not the case.
I think I'd phrase it something like: *Improvements to rendering of full output format.
I'm also a bit torn on the blog-like write-up. The prominence of the change now suggests that this should have been part of a minor release. On the other hand, I do think it's useful for users to understand the change.
It's a bit unfortunate that #19900 landed right after the release. It would have allowed us to use it as an example and explain the entire change as part of an improved rendering for F811.
What we could try to make it less sound like a breaking change (which it isn't, because we don't consider the exact output of full to be under semver), is to set the focus on what this will enable in the future (multifile diagnostics too). As far as the format changes go, the only change I would mention is that the file name is now rendered on a separate line.
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.
Updated the headline! (locally)
I know what you mean about the write-up. I think uv includes more text in some of their release notes, but for Ruff this is exclusive to the minor releases, so this might give the wrong idea. I could shorten the section substantially and/or move it to a sub-bullet in ### Other changes if that would help. It would certainly be less prominent:
Other changes
-
...
-
Improve rendering of the
fulloutput format (#19415)
Ruff now uses the same default diagnostic format as ty. Below is an example diff forF401:-unused.py:8:19: F401 [*] `pathlib` imported but unused +F401 [*] `pathlib` imported but unused + --> unused.py:8:19 | 7 | # Unused, _not_ marked as required (due to the alias). 8 | import pathlib as non_alias - | ^^^^^^^^^ F401 + | ^^^^^^^^^ 9 | 10 | # Unused, marked as required. | - = help: Remove unused import: `pathlib` +help: Remove unused import: `pathlib`
For now, the primary change is in the header, but this new representation will allow us to make further additions to Ruff's diagnostics, such as adding sub-diagnostics and multiple annotations to the same snippet.
Or would it be more helpful to omit the diff and only show the new F811 diagnostic? And agreed that it would have been nice to include both, I somehow didn't quite realize that this was going out in the last release and without the F811 PR.
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.
Moving it under Other makes sense to me. It makes it less prominent (while still prominent enough). I would omit the mention of ty, most users won't be familiar with it and it's unclear why it even matters for them.
MichaReiser
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.
Nice, I like this a lot
full output format changes to the changelog
Summary
I thought this might warrant a small blog-style writeup, especially since we already got a question about it (#19966), but I'm happy to switch back to a one-liner under
### Other changesif preferred.I'll copy whatever we add here to the release notes too.
Do we need a note at the top about the late addition?