Refactor i18-checks to use YAML, modularize utils, and fix errors #7

OmarAI2003 · 2025-02-07T08:23:11Z

Contributor checklist

This pull request is on a separate branch and not the main branch

Description

This PR refactors the i18n check scripts by improving modularity, introducing YAML-based configuration, and ensuring full project functionality.

Refactored Checks: Extracted reusable functions from various check scripts into [utils.py](http://utils.py/) to improve maintainability.
YAML-Based Configuration: Replaced direct directory reads with paths defined in .i18n-check.yaml, making the scripts more flexible.
Bug Fixes: Addressed various issues in check scripts to ensure they function correctly.
Utility Functions: Added helper functions for i18n key validation and YAML configuration loading.
i18n Key Mapping: Implemented gen_map_obj.py to generate an i18n-map.ts file with structured key mappings.
Configuration Update: Added types_dir path in .i18n-check.yaml to ensure scripts locate the correct directories.

Related issue

Move reusable code from scripts into utility.py #6

…e correct directory

…y file

…ng direct directory reads with YAML-configured paths, and fixing bugs so the project initially works.

github-actions · 2025-02-07T08:23:37Z

Thank you for the pull request! ❤️

The i18n-check team will do our best to address your contribution as soon as we can. If you're not already a member of our public Matrix community, please consider joining! We'd suggest using Element as your Matrix client, and definitely join the General and Development rooms once you're in. Also consider attending our bi-weekly Saturday developer syncs! It'd be great to meet you 😊

github-actions · 2025-02-07T08:23:38Z

Maintainer Checklist

The following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :)

The changelog has been updated with a description of the changes for the upcoming release and the corresponding issue (if necessary)

andrewtavis · 2025-02-07T10:02:09Z

Nice, @OmarAI2003! First glance, this is looking really great 😊 I'll try to bring this in before the sync tomorrow and then we can discuss next steps!

andrewtavis

Writing out some notes as I go through the changes, @OmarAI2003:

I'd suggest getting your IDE set up with the ruff (edit: ruff, not black) code formatter and also to install the extension (if you already have this done, then check your settings)
- Imports weren't sorted and there were some formatting changes that can happen automatically on save
- Set formatting to run on save, if you don't have that, and also on paste so that code you get from elsewhere is automatically formatted

andrewtavis · 2025-02-08T10:48:26Z

Installing ruff will also lint the code, which will tell you when you have unused imports and other things like that :)

andrewtavis

This is looking great so far, @OmarAI2003 :) Can I ask you to look through the comments I made and the changes I sent along in my commit so you can familiarize yourself with the changes? 😊 I'll make a suggestion on which issue to work on next in the issues 🚀

andrewtavis · 2025-02-08T10:44:10Z

src/i18n_check/checks/check_key_identifiers.py

-    )
+en_us_json_dict = read_json_file(en_us_json_file)
+
+files_to_check =collect_files(frontend_directory, file_types_to_check, directories_to_skip, files_to_skip)


Let's always do explicit function arguments as it makes it easier for others who don't know what the function does to see what the purpose is. Even if it's just repeating the argument as we use the same name, it still makes it more clear what the argument is being used for :)

andrewtavis · 2025-02-08T10:45:30Z

.i18n-check.yaml

@@ -5,6 +5,7 @@ src-dir: tests
 i18n-dir: tests/test_frontend/test_i18n
 i18n-src: tests/test_frontend/test_i18n/test-i18n-src.json
 i18n-map: tests/test_frontend/test-i18n-map.ts
+types_dir: src\i18n_check\types


I don't think we'll need this, as the i18n-map object location should come directly from the YAML i18n-map value :)

I'll remove this and the file.

The usages of this can all be replaced with i18n_map_file from utils :)

andrewtavis · 2025-02-08T10:53:48Z

src/i18n_check/utils.py

+    """
+    Reads a JSON file and returns its content.
+
+    Parameters:


Let's be sure to follow the formatting for the function docstrings :)

andrewtavis · 2025-02-08T10:55:21Z

src/i18n_check/utils.py

+
+    Parameters
+    ----------
+    p : list str


andrewtavis · 2025-02-08T11:10:39Z

src/i18n_check/utils.py

+i18n_check_dir = str(Path(config["i18n-dir"]).resolve())
+json_file_directory = Path(config["i18n-dir"]).resolve()
+frontend_directory = Path(config["src-dir"]).resolve()
+en_us_json_file = Path(config["i18n-src"]).resolve()


Let's remember that en-US might not be the src language for other projects, and also we might expand this out to work for other file types eventually :) Will rename this accordingly.

andrewtavis · 2025-02-08T11:13:47Z

src/i18n_check/utils.py

+
+# MARK: collecting File
+
+def collect_files(directory, file_types, directories_to_skip, files_to_skip):


Let's also remember to type the arguments for functions :)

andrewtavis · 2025-02-08T11:21:50Z

src/i18n_check/utils.py

+# MARK: YAML Reading
+
+# Define the path to the YAML configuration file
+config_path = Path(__file__).parent.parent.parent / ".i18n-check.yaml"


There's a chance that this is going to be more robust. Specifically we need to figure out how other CLIs find their specific YAML files. The CLI should search the directory where the code is being called, odds are, but we can do this later :)

OmarAI2003 · 2025-02-11T08:42:17Z

src/i18n_check/utils.py

    Parameters
    ----------
-    p : list str
+    p : list[str]
        The string of the directory path of a file that has a key.


I think this part should be changed to

p : str The directory path of a file that contains a key.

Since p is a plain string, not a list. You can see this from the loop variable c in for i, c in enumerate(p):, where c represents individual characters in the inputted p parameter

Ah ok, nice :) Feel free to revert this in the next PR!

OmarAI2003 · 2025-02-11T08:43:19Z

Installing ruff will also lint the code, which will tell you when you have unused imports and other things like that :)

I had Ruff installed as well as its extension, but maybe I wasn't warned by Ruff when I did the commit because I didn't run pre-commit install in the terminal🤔🤷

andrewtavis · 2025-02-11T08:55:13Z

Could be the case, @OmarAI2003 :) Maybe also set up format on save? It'll get figured out eventually :)

OmarAI2003 added 4 commits February 7, 2025 07:12

Add types_dir path in .i18n-check.yaml to ensure scripts read from th…

a727826

…e correct directory

generate i18n-map.ts file with i18n keys mapping by the gen_map_obj.p…

bb41b2c

…y file

Add utility functions for i18n key checks and YAML configuration loading

6ab51e0

Refactor checks by extracting reusable functions to utils.py, replaci…

5a096e7

…ng direct directory reads with YAML-configured paths, and fixing bugs so the project initially works.

Merge branch 'main' into refactor/move-to-utility

0cba7f8

andrewtavis self-requested a review February 7, 2025 09:59

andrewtavis reviewed Feb 8, 2025

View reviewed changes

Explicit args, fix doc strings, switch yaml to kebab, update var names

77ea6cb

andrewtavis approved these changes Feb 8, 2025

View reviewed changes

andrewtavis merged commit 5578742 into activist-org:main Feb 8, 2025

andrewtavis mentioned this pull request Feb 8, 2025

Move reusable code from scripts into utility.py #6

Closed

2 tasks

OmarAI2003 commented Feb 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor i18-checks to use YAML, modularize utils, and fix errors #7

Refactor i18-checks to use YAML, modularize utils, and fix errors #7

OmarAI2003 commented Feb 7, 2025

github-actions bot commented Feb 7, 2025

github-actions bot commented Feb 7, 2025 •

edited by andrewtavis

Loading

andrewtavis commented Feb 7, 2025

andrewtavis left a comment •

edited

Loading

andrewtavis commented Feb 8, 2025

andrewtavis left a comment

andrewtavis Feb 8, 2025

andrewtavis Feb 8, 2025

andrewtavis Feb 8, 2025

andrewtavis Feb 8, 2025

andrewtavis Feb 8, 2025

andrewtavis Feb 8, 2025

andrewtavis Feb 8, 2025

andrewtavis Feb 8, 2025

andrewtavis Feb 8, 2025

OmarAI2003 Feb 11, 2025

andrewtavis Feb 11, 2025

OmarAI2003 commented Feb 11, 2025

andrewtavis commented Feb 11, 2025


		# MARK: collecting File

		def collect_files(directory, file_types, directories_to_skip, files_to_skip):

Refactor i18-checks to use YAML, modularize utils, and fix errors #7

Refactor i18-checks to use YAML, modularize utils, and fix errors #7

Conversation

OmarAI2003 commented Feb 7, 2025

Contributor checklist

Description

Related issue

github-actions bot commented Feb 7, 2025

Thank you for the pull request! ❤️

github-actions bot commented Feb 7, 2025 • edited by andrewtavis Loading

Maintainer Checklist

andrewtavis commented Feb 7, 2025

andrewtavis left a comment • edited Loading

Choose a reason for hiding this comment

andrewtavis commented Feb 8, 2025

andrewtavis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OmarAI2003 commented Feb 11, 2025

andrewtavis commented Feb 11, 2025

github-actions bot commented Feb 7, 2025 •

edited by andrewtavis

Loading

andrewtavis left a comment •

edited

Loading