chore: prepare 3.0.0 release - #1779
Conversation
grantmcdermott
left a comment
There was a problem hiding this comment.
I'd already started working on my own CHANGELOG updates and just incorporated them more or less directly into yours. You might want to scan over, but I think it looks good as-is (certainly much more readable and clearer to follow than it was before).
|
Thank you, but I was going to wait for reviews from others before merging. |
eitsupi
left a comment
There was a problem hiding this comment.
ChatGPT-generated review, based on comparing the current CHANGELOG against the 3.0.0 implementation and manifest behavior.
I noticed three user-facing wording issues where the current CHANGELOG is broader than the implementation:
-
r.plot.useHttpgdis not a simple behavior-preserving rename.
The "Renamed settings" section currently says that the deprecated names still work and that existing configurations will continue to behave as before. That is not true forr.plot.useHttpgd: false: in 2.8.8,falseselected the standard viewer, while in 3.0.0 the legacy setting only has an effect when it istrue; otherwise backend resolution falls back toauto, which may select jgd or httpgd if installed.A more precise migration note would be that
r.plot.useHttpgd: truecontinues to select httpgd whiler.plot.backendremainsauto, and users who want to preserve the oldfalsebehavior should setr.plot.backend: "standard"explicitly. -
Automatic reconnect applies to managed R terminals/sessions.
The Session section currently says: "R terminals now reconnect automatically after reloading the VS Code window." The reconnect logic in #1764 is for managed terminals with vscode-R discovery metadata. "Managed R terminals now reconnect automatically..." would more accurately describe the behavior and avoid implying that arbitrary externally started R processes are automatically recovered. -
The console fallback description omits the legacy
r.rterm.<platform>precedence.
The CHANGELOG currently says: "Ifr.consolePathis unset,r.executablePathis used for the console too." The actual resolution order isr.consolePath→ legacyr.rterm.<platform>→ explicitly configuredr.executablePath→ system R.This could be clarified as: "If neither
r.consolePathnor the legacyr.rterm.<platform>setting is set, an explicitly configuredr.executablePathis used for the console too."
These are documentation accuracy issues only, but tightening the wording would help users migrating existing configurations to 3.0.0.
To clarify: this was a comment about the changelog before your (and my) refactor. I wasn't trying to say my incremental changes were much better than yours... |
|
I see. To clarify, though, that wording wasn't what disappointed me. What concerned me was that you pushed substantial changes directly to my PR branch and then approved and merged the PR without getting agreement or review from anyone else. More generally, if someone has both permission to modify another person's PR and permission to approve it, this workflow means they can effectively introduce and approve their own changes without any independent review. That seems to undermine the purpose of requiring an approval in the first place. So I think this may actually be something we should address in the repository settings. |
|
TBC I agree too. But in my defense for this particular case, we had an unfortunate sequence of timing / competing PRs after I said I would submit one for #1769, and interpreted the subsequent comment about merging these together as tacit approval. (Preempted, in turn, by an earlier request not to block merges through minor revision requests... Which shaped my prior.) I would obviously not do this for material code changes. But it's good to set form collaboration norms going forward. |

Summary
Prepare the v3.0.0 release by replacing the release-candidate version and organizing the changelog around changes relevant to users upgrading from 2.8.8. Keep the README overview concise and point to the changelog for upgrade details.
Changes
Closes #1769