Add custom linter for i18n/label translation keywords #17610
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ilmotta
requested review from
cammellos,
vkjr,
FFFra,
flexsurfer,
yqrashawn,
Parveshdhull,
ajayesivan,
smohamedjavid,
OmarBasem and
mohsen-ghafouri
ilmotta
commented
Oct 10, 2023
| @@ -0,0 +1,25 @@ | |||
| (ns hooks.core | |||
There was a problem hiding this comment.
FYI: this is the namespace that matters for this PR.
Jenkins BuildsClick to see older builds (4)
|
ilmotta
requested review from
briansztamfater,
J-Son89,
BalogunofAfrica and
siddarthkay
yqrashawn
approved these changes
Oct 11, 2023
Parveshdhull
approved these changes
Oct 11, 2023
siddarthkay
approved these changes
Oct 11, 2023
|
hey @ilmotta awesome work, do you think it will be possible to analyze en.json, and show the info about, unused keys and absent keys in en.json? thanks |
mohsen-ghafouri
approved these changes
Oct 11, 2023
84% of end-end tests have passedFailed tests (7)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (36)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
|
OmarBasem
approved these changes
Oct 11, 2023
BalogunofAfrica
approved these changes
Oct 11, 2023
Thanks @flexsurfer. It's theoretically possible because clj-kondo has a project-wide analyzer, different than the solution I used here for linting purposes. We can explore your idea I'm sure. |
ilmotta
force-pushed
the
ilmotta/add-custom-status-linter
branch
from
c2cb287 to
37db916
Compare
J-Son89
approved these changes
Oct 11, 2023
ilmotta
force-pushed
the
ilmotta/add-custom-status-linter
branch
from
37db916 to
9db641f
Compare
Closed
This was referenced Nov 3, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Have you ever made a comment about the guideline: "Please, qualify translation keywords with
:t" in calls toi18n/label, or something like that? I assure you this kind of comment has been made numerous times!What if we could lint and automate this verification? Of course we can, with the help of Clojure's linter, clj-kondo.
In this PR I'm adding a custom linter to verify
i18n/labelis called with a qualified keyword, like:t/foo. I chose to implement this simple linter to avoid increasing the scope of this PR, but the sky is the limit 🛸And?
I hope this PR sets the stage for other devs to consider more and more lint automation instead of manually reviewing conventions in PRs.
If you want to understand how to write custom linters, check out https://github.com/clj-kondo/clj-kondo/blob/master/doc/hooks.md. You can fire the Clojure JVM REPL in
status-mobileand play with the clj-kondo hook too, it works beautifully.Why do we care about this rule?
By making sure all translation keywords are qualified with
:t, it is trivial to grep or replace them because they're unique in the repo, and can't be confused with other words if you search by:t/<something>.Notes
It's a best practice to commit clj-kondo configuration from external libs in the
.clj-kondodirectory. The directory.clj-kondo/babashkais auto-generated, that's why I added it.Screenshots
I didn't check Vim or IntelliJ, but I'm sure they work just as well with no extra configuration on your part. Just remember to restart the language server or to delete the
clj-kondo/.cachedirectory if you're not seeing errors in the editor for this new linter.Emacs:
VS Code:
Areas that may be impacted
None, safe keyword updates.
Steps to test
For developers only: remove the
:tqualification in any translation keyword passed toutils.i18n/label. Then check the output ofmake lintand the flagged error in your editor.status: ready