Skip to content
This repository has been archived by the owner on Apr 14, 2023. It is now read-only.

Switch back setting internal id to vale.valeCLI.path #91

Closed
wants to merge 1 commit into from

Conversation

apupier
Copy link
Contributor

@apupier apupier commented Jul 29, 2022

The interest of switching back is that it will avoid end-users having a
setting marked as invalid when opening the setting file. (or to
implement a migration of settings)

fixes #90

another advanateg is that ti will keep a group for Vale CLI binary which will be useful for implementing #75

The interest of switching back is that it will avoid end-users having a
setting marked as invalid when opening the setting file. (or to
implement a migration of settings)

fixes errata-ai#90

Signed-off-by: Aurélien Pupier <apupier@redhat.com>
@apupier apupier mentioned this pull request Jul 29, 2022
@jdkato
Copy link
Member

jdkato commented Aug 7, 2022

I think we should move all settings back to the vale.valeCLI.* convention. Generally, I think not breaking things or forcing users to update otherwise unmodified settings is the goal.

Thoughts, @ChrisChinchilla?

@ChrisChinchilla
Copy link
Collaborator

Hmm @jdkato I have a few thoughts…

We did break things before when initially merging the extensions, albeit with a lot less users. I feel like the legacy of that back and forth is potentially confusing to people, but it's not a deal breaker.

My biggest concern is that maybe people have already migrated to the new settings and then we break things all over again. Let me quickly investigate if there's some sort of alias option available for VS Code settings, and if not, then we'll switch back.

@jdkato
Copy link
Member

jdkato commented Aug 8, 2022

My biggest concern is that maybe people have already migrated to the new settings and then we break things all over again.

This is a good point, but I think part of the problem is that the new settings aren't working because some of the old IDs are left in the source code (such as vale.valeCLI.minAlertLevel). So, I think we should be safe there.

@jdkato jdkato mentioned this pull request Aug 9, 2022
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.

[regression]Specifying vale cli path in settings is no more working with 0.15.0
3 participants