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.

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

Added use snippet body as a description when none is provided #181115 #181381

Merged
merged 6 commits into from
May 10, 2023
Merged

Added use snippet body as a description when none is provided #181115 #181381

merged 6 commits into from
May 10, 2023

Conversation

TheSylvester
Copy link
Contributor

@TheSylvester TheSylvester commented May 3, 2023

edit: added word "implemented"

TheSylvester and others added 2 commits May 3, 2023 01:41
- use snippet body as a description when none is provided #181115
- editor.snippets.fillDescriptions.enabled toggle in settings
@TheSylvester
Copy link
Contributor Author

@microsoft-github-policy-service agree

@jrieken jrieken added this to the May 2023 milestone May 3, 2023
@jrieken
Copy link
Member

jrieken commented May 3, 2023

@TheSylvester Thanks. This is already looking pretty good.

editor.snippets.fillDescriptions.enabled toggle in settings

I would like to disagree with that and implement this w/o setting.

@TheSylvester
Copy link
Contributor Author

editor.snippets.fillDescriptions.enabled toggle in settings

I would like to disagree with that and implement this w/o setting.

I should clarify that I implemented a setting to toggle this to be enabled. By default I've set this to be false to be optionally enabled, unless you see something different?

@jrieken
Copy link
Member

jrieken commented May 3, 2023

I should clarify that I implemented a setting to toggle this to be enabled. By default I've set this to be false to be optionally enabled, unless you see something different?

That's what I see but I am saying that a setting isn't needed. This is an improvement and shouldn't be behind a setting but just happen

@TheSylvester
Copy link
Contributor Author

I see! I was just following the ethos of making as few changes as possible, especially to default behaviour. Just accounting for folks who would appreciate being able to see their snippet without the description (maybe that's why they didn't put one in).

There's definitely a case for making this default true but I leave that up to the maintainers.

@jrieken
Copy link
Member

jrieken commented May 9, 2023

I was just following the ethos of making as few changes as possible, especially to default behaviour

We take improvement without setting, so please remove that part in the PR

@TheSylvester
Copy link
Contributor Author

Thank you for confirming. I've made the required changes. Kindly let me know if there's anything else required.

Copy link
Member

@jrieken jrieken left a comment

Choose a reason for hiding this comment

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

Thanks

@jrieken jrieken merged commit 3242ce2 into microsoft:main May 10, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jun 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants