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

VOC-406 Introduces LocalesWithText as a way to abstract away from Map<String,String> #440

Merged
merged 6 commits into from
Nov 21, 2023

Conversation

IanCrossCD
Copy link
Contributor

Description:

  • Introduces LocalesWithText as a way to abstract away from Map<String,String>.

  • Allows us to move away from a Util and use methods and extensions instead.

  • Organizes a few util package files into locale package.

  • No new DB manipulation should be necessary since its transformed from a string in both scenarios.

  • Acceptance Criteria satisfied

  • Regression Testing

…ling with Map<String, String>. Allows us to move away from a Util and use methods and extensions instead. Organizes a few util package files into locale package
@IanCrossCD
Copy link
Contributor Author

I want to move away from using LocaleString all together but Locale was not friendly to conversions in my 10 minute attempt. I could at least strip it from Domain and below if our models had separation :)

@IanCrossCD IanCrossCD linked an issue Nov 14, 2023 that may be closed by this pull request
}

}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Dang, Moshi doesn't play nice with typealiases out of the box?

/**
* Sets the string corresponding to the given localeString.
* @param localeString
* @param text
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there value in including @param tags that have no further description?

Copy link
Collaborator

@PaulKlauser PaulKlauser left a comment

Choose a reason for hiding this comment

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

LGTM Thanks!

Try to keep an eye out for ways to break up PRs that touch 42 files. That can be by having multiple commits within the PR, where the big "name change" commits are isolated, or spreading the work across multiple PRs 😁

@IanCrossCD IanCrossCD merged commit fa87dec into main Nov 21, 2023
1 check passed
@IanCrossCD IanCrossCD deleted the VOC-406/make-localized-types-more-clear branch November 21, 2023 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make localized types more clear
2 participants