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

chore(style): Sort JSON keys in translations/en.json #20785

Merged
merged 2 commits into from
Jul 22, 2024

Conversation

ilmotta
Copy link
Contributor

@ilmotta ilmotta commented Jul 17, 2024

Summary

Quick win PR, this one now sorts keys (non-recursively) in translations/en.json. The solution uses the prettier plugin https://github.com/Gudahtt/prettier-plugin-sort-json.

I opted to only format translations/en.json because that's the only file we need to change manually and this speeds up a little bit our already slow lint and lint-fix make targets.

  • Dependency added: prettier-plugin-sort-json version 4.0.0
  • Dependency upgraded: prettier, from version 2.8.8 to 3.3.3.
  1. Sorted keys should reduce conflicts a little bit, because many conflicts happen because CCs add translations at the end of the file.
  2. Having keys sorted helps with readability because similarly worded translations are close. Also, sometimes the dev doesn't know exactly where a key is and doesn't remember the name precisely.
  3. Sorting as a rule should also help all CCs know where to add keys in en.json without thinking too much.

Areas that may be impacted

None. I compared my manual sorting with the output from the lib, and no keys were removed for example. Seems to be a robust solution.

Steps to test

make lint or make lint-fix

status: ready

@@ -317,7 +317,7 @@ lint: ##@test Run code style checks
scripts/lint/translations.clj && \
zprint '{:search-config? true}' -sfc $$ALL_CLOJURE_FILES && \
sh scripts/lint/trailing-newline.sh && \
node_modules/.bin/prettier --write .
node_modules/.bin/prettier --check .
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The lint target shouldn't change files, this should be reserved to the lint-fix target.

@@ -97,7 +97,8 @@
"jest-silent-reporter": "^0.5.0",
"nodemon": "^2.0.16",
"nyc": "^14.1.1",
"prettier": "^2.8.8",
"prettier": "^3.3.3",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to upgrade prettier to latest (above 3.x) because the JSON sort plugin has this requirement.

@status-im-auto
Copy link
Member

status-im-auto commented Jul 17, 2024

Jenkins Builds

Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ afbc518 #2 2024-07-17 13:32:06 ~10 min ios 📱ipa 📲
✔️ afbc518 #2 2024-07-17 13:33:27 ~11 min tests 📄log
✔️ afbc518 #2 2024-07-17 13:35:20 ~13 min android-e2e 🤖apk 📲
✔️ afbc518 #2 2024-07-17 13:35:27 ~13 min android 🤖apk 📲
✔️ afc15cf #3 2024-07-22 00:46:33 ~6 min tests 📄log
✔️ afc15cf #3 2024-07-22 00:46:40 ~6 min android-e2e 🤖apk 📲
✔️ afc15cf #3 2024-07-22 00:48:51 ~8 min android 🤖apk 📲
✔️ afc15cf #3 2024-07-22 00:50:25 ~10 min ios 📱ipa 📲

Copy link
Member

@briansztamfater briansztamfater left a comment

Choose a reason for hiding this comment

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

Wohoo! Thanks for this :)

@status-im-auto
Copy link
Member

86% of end-end tests have passed

Total executed tests: 7
Failed tests: 0
Expected to fail tests: 1
Passed tests: 6
IDs of expected to fail tests: 727232 

Expected to fail tests (1)

Click to expand

Class TestWalletOneDevice:

1. test_wallet_add_remove_watch_only_account, id: 727232

Device 1: Find `EditBox` by `accessibility id`: `add-address-to-watch`
Device 1: Type `0x8d2413447ff297d30bdc475f6d5cb00254685aae` to `EditBox`

critical/test_wallet.py:249: in test_wallet_add_remove_watch_only_account
    self.wallet_view.add_watch_only_account(address=address_to_watch, account_name=new_account_name)
../views/wallet_view.py:163: in add_watch_only_account
    self.account_has_activity_label.wait_for_visibility_of_element()
../views/base_element.py:147: in wait_for_visibility_of_element
    raise TimeoutException(
 Device 1: Text by accessibility id:`account-has-activity` is not found on the screen after wait_for_visibility_of_element 

[[Missing networks in account address, https://github.com//issues/20166]]

Device sessions

Passed tests (6)

Click to expand

Class TestWalletOneDevice:

1. test_wallet_add_remove_regular_account, id: 727231
Device sessions

Class TestCommunityOneDeviceMerged:

1. test_restore_multiaccount_with_waku_backup_remove_switch, id: 703133
Device sessions

2. test_community_copy_and_paste_message_in_chat_input, id: 702742
Device sessions

Class TestWalletMultipleDevice:

1. test_wallet_send_asset_from_drawer, id: 727230
2. test_wallet_send_eth, id: 727229

Class TestOneToOneChatMultipleSharedDevicesNewUi:

1. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
Device sessions

@ilmotta ilmotta merged commit cec46f9 into develop Jul 22, 2024
6 checks passed
@ilmotta ilmotta deleted the ilmotta/sort-json-keys branch July 22, 2024 01:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: DONE
Development

Successfully merging this pull request may close these issues.

None yet

7 participants