Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Toggle Comments Inconsistent Behavior With comments.visible Setting #167722

Merged
merged 1 commit into from Nov 30, 2022
Merged

Toggle Comments Inconsistent Behavior With comments.visible Setting #167722

merged 1 commit into from Nov 30, 2022

Conversation

alexr00
Copy link
Member

@alexr00 alexr00 commented Nov 30, 2022

Fixes #167663

@alexr00 alexr00 enabled auto-merge (squash) November 30, 2022 13:58
@alexr00 alexr00 self-assigned this Nov 30, 2022
@VSCodeTriageBot VSCodeTriageBot added this to the November 2022 milestone Nov 30, 2022
Tyriar
Tyriar reviewed Nov 30, 2022
View reviewed changes
Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this meets the new severity bar for fixes after testing has occurred? Should we merge this for December instead?

src/vs/workbench/contrib/comments/browser/commentService.ts Show resolved Hide resolved
@alexr00
Copy link
Member Author

alexr00 commented Nov 30, 2022 •
edited

I'm not sure this meets the new severity bar for fixes after testing has occurred? Should we merge this for December instead?

This is a bug in a new feature that was discovered during testing. We are not doing candidates only at this time, so this meets the fix bar.

@aeschli in case you feel differently.

Tyriar
Tyriar approved these changes Nov 30, 2022
View reviewed changes
Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree if it's going to be mentioned in the release notes, it looked like a minor bug just reading the report is all.

@alexr00 alexr00 merged commit 5c79f5c into main Nov 30, 2022
@alexr00 alexr00 deleted the alexr00/issue167663 branch November 30, 2022 14:14
@alexr00
Copy link
Member Author

alexr00 commented Nov 30, 2022

The new feature is the comments.visible setting and the bug is between the interaction with this setting and the existing command "Comments: Toggle Editor Commenting". Part of the selling point for the setting is the way it interacts with the command (similar to View: Toggle Word Wrap).

@github-actions github-actions bot locked and limited conversation to collaborators Jan 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Tyriar Tyriar Tyriar approved these changes

Assignees

@alexr00 alexr00

Labels
None yet
Projects
None yet
Milestone
November 2022
Development

Successfully merging this pull request may close these issues.

Toggle Comments Inconsistent Behavior With comments.visible Setting
3 participants
@alexr00 @Tyriar @VSCodeTriageBot