Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Use & enforce snake_case naming convention on config.json settings #8062

Merged
merged 9 commits into from
Mar 18, 2022

Conversation

turt2live
Copy link
Member

@turt2live turt2live commented Mar 15, 2022

Per internal vote.

Review with element-hq/element-web#21429

This change does:

  • Rename ConfigOptions to IConfigOptions to match code convention/style
  • Update comments and surrounding documentation
  • Define every single option
  • Enable a linter to enforce the convention
  • Move IConfigOptions (and directly related interfaces) to a dedicated file so the linter rules can be applied
  • Invent a translation layer for using the new settings with fallbacks (SnakedObject)

This change is marked as an internal change (Task), so will not be included in the changelog.

Preview: https://pr8062--matrix-react-sdk.netlify.app
⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.

@turt2live turt2live self-assigned this Mar 15, 2022
@turt2live turt2live force-pushed the travis/sssssssnake_case_config branch from 317adcb to f4b71d2 Compare March 15, 2022 22:41
@turt2live turt2live added the T-Task Refactoring, enabling or disabling functionality, other engineering tasks label Mar 16, 2022
@turt2live turt2live force-pushed the travis/sssssssnake_case_config branch from f4b71d2 to 6a94b31 Compare March 16, 2022 01:39
This change:
* Rename `ConfigOptions` to `IConfigOptions` to match code convention/style, plus move it to a dedicated file
* Update comments and surrounding documentation
* Define every single documented option (from element-web's config.md)
* Enable a linter to enforce the convention
* Invent a translation layer for a different change to use
* No attempt to fix build errors from doing this (at this stage)
@turt2live turt2live force-pushed the travis/sssssssnake_case_config branch from 6a94b31 to 0109e34 Compare March 16, 2022 01:42
@turt2live
Copy link
Member Author

da217d2 demonstrates the lint rule working:

image

@turt2live turt2live force-pushed the travis/sssssssnake_case_config branch from bb327f7 to 4b320d0 Compare March 16, 2022 03:27
@codecov
Copy link

codecov bot commented Mar 16, 2022

Codecov Report

Merging #8062 (04b77db) into develop (4e4ce65) will increase coverage by 0.00%.
The diff coverage is 27.70%.

@@           Coverage Diff            @@
##           develop    #8062   +/-   ##
========================================
  Coverage    27.12%   27.13%           
========================================
  Files          868      869    +1     
  Lines        52097    52137   +40     
  Branches     13199    13211   +12     
========================================
+ Hits         14130    14145   +15     
- Misses       37967    37992   +25     
Impacted Files Coverage Δ
src/Analytics.tsx 16.76% <0.00%> (-0.96%) ⬇️
src/BasePlatform.ts 2.51% <ø> (ø)
src/KeyBindingsDefaults.ts 54.76% <0.00%> (ø)
src/Livestream.ts 8.33% <0.00%> (ø)
src/components/structures/HomePage.tsx 18.18% <0.00%> (+1.03%) ⬆️
src/components/structures/HostSignupAction.tsx 0.00% <0.00%> (ø)
src/components/structures/LoggedInView.tsx 0.69% <ø> (ø)
src/components/structures/MatrixChat.tsx 0.44% <0.00%> (-0.01%) ⬇️
src/components/structures/RoomDirectory.tsx 1.24% <0.00%> (ø)
src/components/structures/UserMenu.tsx 26.34% <0.00%> (+0.14%) ⬆️
... and 37 more

@turt2live turt2live marked this pull request as ready for review March 18, 2022 03:13
@turt2live turt2live requested a review from a team as a code owner March 18, 2022 03:13
@turt2live turt2live removed their assignment Mar 18, 2022
Copy link
Contributor

@germain-gg germain-gg left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM

{
...
"settingDefaults": {
"setting_defaults": {
"settingName": true
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"settingName": true
"setting_name": true

Copy link
Member Author

Choose a reason for hiding this comment

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

The settings can still be camel case, and most are, unfortunately.

@turt2live turt2live merged commit d8a939d into develop Mar 18, 2022
@turt2live turt2live deleted the travis/sssssssnake_case_config branch March 18, 2022 16:12
benbz added a commit to element-hq/element-web that referenced this pull request Jul 17, 2023
As of matrix-org/matrix-react-sdk#8062 / #21429 this should have been snake case rather than camelCase
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Task Refactoring, enabling or disabling functionality, other engineering tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants