Model Setup for String Catalog by Uni-boy · Pull Request #14 · superarts/XStringCatalog

Uni-boy · 2023-12-19T01:51:13Z

Pull Request: XCString Model Setup, Add Tests, Other Fixes

Description

This pull request introduces the XCString model to our codebase.

The primary goal is to enhance our application's localization capabilities, making it easier to manage and use localized strings.

Related Issue

#1

Changes Made

Model Creation: Implemented the XCString model, designed to encapsulate localization logic and provide a more structured approach to handling localized strings.

Decoding Logic: Added functionality to decode localization data from JSON, enabling dynamic localization based on user preferences or system settings.

Unit Tests: Included comprehensive unit tests to validate the functionality of the XCString model, particularly focusing on its ability to correctly handle various localization scenarios.

Checklist

I have added appropriate comments/documentation.

All existing tests pass, and I have added new tests.

The commit messages are clear and follow best practices.

Additional Comments

I implemented the model according to the Json data listed in issue, but the actual json is more complex, so I commented out the model made for those extra attributes.

superarts · 2023-12-19T03:38:27Z

@Uni-boy is your PR ready for review please?

Uni-boy · 2023-12-19T07:21:32Z

Yeah my PR is ready.

superarts

Why would we just add some passive data models without using them?

I recommend taking a look at closed PRs. See what kind of PRs are merged and what kind of PRs are rejected.

superarts · 2023-12-19T07:27:43Z

+    let strings: [String: StringInfo]
+}
+
+struct StringInfo: Decodable {


Is it the best practice to put all structs in the same source file?

superarts

Again, these structs should be consumed. Please check my previous comments: #14 (review)

superarts · 2023-12-28T11:16:17Z

@Uni-boy if you don't have any plan to update this PR, please feel free to close it. Thank you.

Uni-boy · 2023-12-31T05:17:50Z

I finalized the model and all tests passed.

superarts · 2024-01-01T15:58:31Z

I finalized the model and all tests passed.

Please merge from master and resolve conflicts.

More importantly, when I said "these structs should be consumed", I meant the structs you created should replace codes like let strings = json["strings"] as? [String: Any] in main.swift. What do you think the codebase can benefit from some data models that are not used anywhere?

…to feature/create-decodable-models

…ument

Uni-boy · 2024-01-03T00:28:01Z

I finalized the model and all tests passed.

Please merge from master and resolve conflicts.

More importantly, when I said "these structs should be consumed", I meant the structs you created should replace codes like let strings = json["strings"] as? [String: Any] in main.swift. What do you think the codebase can benefit from some data models that are not used anywhere?

I applied the model to main.swift and solved the TODO in StringEnumHelper.swift

superarts

Please address the obvious issues I pointed out, but my PR review is not finished yet. I'll add another review soon.

superarts · 2024-01-04T19:59:35Z

Revert this?

superarts · 2024-01-04T19:59:59Z

Delete xcode related files from git. You shouldn't use xcode for this project, but if you insist doing so (not recommended, I can see a sample xcode project being added in future, but not for the CLI tool itself), you can add this file to .gitignore.

superarts · 2024-01-04T20:08:07Z

+    // let variations: Variations?
+}
+
+// struct Variations: Decodable {


These are dead codes too but I'll need to further look into it.

superarts

Almost there! Just a few minor suggestions.

superarts

Please address this: #14 (comment)

And added some other suggestions regarding unit tests. Sorry I forgot them in my previous review.

superarts · 2024-01-05T04:48:23Z

+                let decoder = JSONDecoder()
+                expect{
+                    try decoder.decode(Localizations.self, from: jsonData)
+                }.toNot(throwError())


One more thing: could you add some to(equal()) evaluation please?

superarts · 2024-01-05T04:48:49Z

+                    },
+                    "version" : "1.0"
+                }
+                """


superarts · 2024-01-05T05:12:42Z

+
+                    // Verify the contents of 'strings' dictionary
+                    expect(decodedData.strings["Login"]).toNot(beNil())
+                    expect(decodedData.strings["Login"]?.localizations?["en"]?.stringUnit?.state).to(equal("translated"))


Eventually we want to use Decodable for all these JSON objects, but this PR is good as is.

set XCStrings model

ea3a79a

superarts requested changes Dec 19, 2023

View reviewed changes

Fix comments in model.swift

eb4bb9f

Uni-boy requested a review from superarts December 19, 2023 17:05

superarts requested changes Dec 20, 2023

View reviewed changes

Uni-boy added 7 commits December 30, 2023 04:00

Model Finalized

f47ceb8

modification on model

693ed27

XCStrings Model finalized

428022b

issue number added

7667b94

modify model file position

426a6f3

Model test added

b97197c

modify models permission

f76beda

Uni-boy requested a review from superarts December 31, 2023 05:13

Uni-boy added 3 commits January 1, 2024 11:27

Merge branch 'master' of https://github.com/Uni-boy/XStringCatalog in…

b2a2ed6

…to feature/create-decodable-models

Test and Package.swift conflicts resolved

100669c

model applied to main.swift & value extracted and added as inline doc…

619bc03

…ument

Uni-boy added 2 commits January 2, 2024 19:50

Add test to ensure model correctly decodes JSON data

1b35b21

refactor hard coded part in StringEnumHelper

2f33359

Uni-boy changed the title ~~set XCStrings model~~ Model Setup for String Catalog Jan 3, 2024

superarts reviewed Jan 3, 2024

View reviewed changes

Comment thread StringCatalogEnum/Sources/StringCatalogEnum/main.swift

modify file structure

0d70803

Uni-boy requested a review from superarts January 3, 2024 22:24

Uni-boy added 2 commits January 3, 2024 20:10

cancel error handling

30b0a04

Modified StringEnumLibrary to correctly generate inline document

4b64da9

superarts requested changes Jan 4, 2024

View reviewed changes

removed unrelated files and dead codes

5ea8aeb

Uni-boy requested a review from superarts January 4, 2024 21:41

superarts requested changes Jan 5, 2024

View reviewed changes

Comment thread StringCatalogEnum/Sources/StringCatalogEnumLibrary/StringEnumHelper.swift Outdated

Comment thread StringCatalogEnum/Sources/StringCatalogEnumLibrary/StringEnumHelper.swift Outdated

Comment thread StringCatalogEnum/Sources/StringCatalogEnumLibrary/StringEnumHelper.swift Outdated

superarts assigned Uni-boy Jan 5, 2024

superarts added the good first issue Good for newcomers label Jan 5, 2024

minor modification & removed dead codes

686a605

Uni-boy requested a review from superarts January 5, 2024 04:45

superarts requested changes Jan 5, 2024

View reviewed changes

added some tests

4ec68fe

Uni-boy requested a review from superarts January 5, 2024 05:11

Apply suggestions from code review

159ef88

superarts approved these changes Jan 5, 2024

View reviewed changes

superarts merged commit e359dd0 into superarts:master Jan 5, 2024

superarts mentioned this pull request Jan 5, 2024

StringCatalogEnum: Create Decodable Models to decode xcstrings JSON file #1

Open

Conversation

Uni-boy commented Dec 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request: XCString Model Setup, Add Tests, Other Fixes

Description

Related Issue

Changes Made

Checklist

Additional Comments

Uh oh!

superarts commented Dec 19, 2023

Uh oh!

Uni-boy commented Dec 19, 2023

Uh oh!

superarts left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

superarts Dec 19, 2023

Choose a reason for hiding this comment

Uh oh!

superarts left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

superarts commented Dec 28, 2023

Uh oh!

Uni-boy commented Dec 31, 2023

Uh oh!

superarts commented Jan 1, 2024

Uh oh!

Uni-boy commented Jan 3, 2024

Uh oh!

Uh oh!

superarts left a comment

Choose a reason for hiding this comment

Uh oh!

superarts Jan 4, 2024

Choose a reason for hiding this comment

Uh oh!

superarts Jan 4, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

superarts Jan 4, 2024

Choose a reason for hiding this comment

Uh oh!

superarts left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

superarts left a comment

Choose a reason for hiding this comment

Uh oh!

superarts Jan 5, 2024

Choose a reason for hiding this comment

Uh oh!

superarts Jan 5, 2024

Choose a reason for hiding this comment

Uh oh!

superarts Jan 5, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uni-boy commented Dec 19, 2023 •

edited

Loading

superarts left a comment •

edited

Loading

superarts left a comment •

edited

Loading