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

refactor: Add TranslationFacility for decoupling from HOVerwaltung #2143

Merged

Conversation

sgcr
Copy link
Contributor

@sgcr sgcr commented Aug 28, 2024

  1. changes proposed in this pull request:
  • In the PR [FEATURE]: #2140: Add ArenaInfoPanel #2141 for feature [FEATURE] Arena sizer: Add the general information about the current stadium #2140 (ArenaInfoPanel) were some comments. So I started developing a HumanDuration class that could also produce a human readable string from itself.
    That needed translations, and when I wrote tests, I noticed that I needed an instance of HOVerwaltung, which is a singleton. I got a strange error message that the DB is working and the reason is the singleton.
    Those are the facts.
    The idea is to have a holder of the globally used instance of the translator: TranslationFacility
    The main advantage is that it is possible to change the translator of the application during the tests (e.g., the LANGUAGE_NO_TRANSLATION or LANGUAGE_DEFAULT) without doing anything else by accident.
  1. src/main/resources/release_notes.md ...
  • has been updated
  1. [Optional] suggested person to review this PR @tychobrailleur @wsbrenk

@sgcr sgcr force-pushed the feature/refactor-access-to-global-translator branch 3 times, most recently from 873d28f to 35b1976 Compare August 29, 2024 19:58
@sgcr sgcr force-pushed the feature/refactor-access-to-global-translator branch 2 times, most recently from 5d9229a to 25d23e0 Compare September 7, 2024 13:21
Copy link
Collaborator

@tychobrailleur tychobrailleur left a comment

Choose a reason for hiding this comment

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

Thanks! Some minor changes and comments, but definitely a change that improves testability. Also, please keep in mind that one of the project goals is to migrate to Kotlin.

src/main/java/core/model/HOVerwaltung.java Show resolved Hide resolved
src/main/java/core/gui/model/ArenaStatistikTableModel.java Outdated Show resolved Hide resolved
src/main/java/core/model/player/MatchRoleID.java Outdated Show resolved Hide resolved
src/main/java/module/lineup/CopyListener.java Show resolved Hide resolved
src/test/java/core/model/TranslatorTest.java Outdated Show resolved Hide resolved
src/test/java/core/model/TranslationFacilityTest.java Outdated Show resolved Hide resolved
@sgcr sgcr force-pushed the feature/refactor-access-to-global-translator branch from 56adb8b to b145d74 Compare September 10, 2024 21:31
@sgcr sgcr force-pushed the feature/refactor-access-to-global-translator branch from 6967f3d to c2d5c14 Compare September 10, 2024 22:16
@sgcr sgcr changed the title Refactor: Add TranslationFacility for decoupling from HOVerwaltung refactor: Add TranslationFacility for decoupling from HOVerwaltung Sep 10, 2024
Copy link
Collaborator

@tychobrailleur tychobrailleur left a comment

Choose a reason for hiding this comment

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

Thanks for the work!

@wsbrenk wsbrenk merged commit 950b01b into ho-dev:master Sep 23, 2024
1 check passed
@sgcr sgcr deleted the feature/refactor-access-to-global-translator branch September 23, 2024 20:50
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.

3 participants