Skip to content

Refactor settings.ts #4235

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

Merged
merged 9 commits into from
Nov 3, 2022
Merged

Refactor settings.ts #4235

merged 9 commits into from
Nov 3, 2022

Conversation

andyleejordan
Copy link
Member

@andyleejordan andyleejordan commented Oct 31, 2022

This adds logging to our changeSetting() function so we can track what settings the extension makes changes to (mostly because we're really confused how editorServicesWaitForDebugger keeps getting turned on). It also finishes a clean up started in the strict-mode refactor to make the settings fields non-optional and uses a recursive function to populate ISettings from the WorkspaceConfiguration, complete with a regression test (which will fail if you run the tests from the launch config and have modified settings).

@andyleejordan andyleejordan added Issue-Enhancement A feature request (enhancement). Area-Logging labels Oct 31, 2022
@andyleejordan andyleejordan requested a review from a team October 31, 2022 20:06
@andyleejordan andyleejordan force-pushed the andschwa/settings branch 4 times, most recently from ac2d6ad to 237fb52 Compare November 2, 2022 18:28
@andyleejordan andyleejordan marked this pull request as draft November 2, 2022 18:28
@andyleejordan andyleejordan force-pushed the andschwa/settings branch 3 times, most recently from 14a9121 to aa7b026 Compare November 2, 2022 21:57
@andyleejordan andyleejordan changed the title Clean up and log settings Refactor settings.ts Nov 2, 2022
@andyleejordan andyleejordan marked this pull request as ready for review November 2, 2022 22:19
@andyleejordan
Copy link
Member Author

Hm, this might break developer.featureFlags since that's a string[].

@andyleejordan andyleejordan force-pushed the andschwa/settings branch 2 times, most recently from 68be906 to 0e37854 Compare November 2, 2022 23:46
This setting was never meant to be changed!
As the client-side folder was removed eons ago.
Copy link
Collaborator

@SeeminglyScience SeeminglyScience left a comment

Choose a reason for hiding this comment

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

LGTM!

@andyleejordan
Copy link
Member Author

Ok let's do it!

@andyleejordan andyleejordan merged commit 0f422a0 into main Nov 3, 2022
@andyleejordan andyleejordan deleted the andschwa/settings branch November 3, 2022 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Logging Issue-Enhancement A feature request (enhancement).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants