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

cleanup: remove createTranslator methods in BaseTest #65

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

timazhum
Copy link

Addressing TODO of removing createTranslator() methods in BaseTest and using createDeepLClient() instead

Addressing TODO of removing createTranslator() methods in BaseTest and using createDeepLClient() instead
@timazhum
Copy link
Author

@JanEbbing @daniel-jones-dev a tiny-tiny clean up for the tests, based on TODO requested

@JanEbbing
Copy link
Member

JanEbbing commented Feb 17, 2025

Thanks for this. I'm a tiny bit hesitant, because now if I ever accidentally make a backwards incompatible change to Translator I have no way of catching that any more? Maybe we should keep some basic tests on Translator, e.g. translateText and translateDocument?

E: Alternatively, we could just release 2.0 and get rid of Translator 🤔

@timazhum
Copy link
Author

Thanks for this. I'm a tiny bit hesitant, because now if I ever accidentally make a backwards incompatible change to Translator I have no way of catching that any more? Maybe we should keep some basic tests on Translator, e.g. translateText and translateDocument?

E: Alternatively, we could just release 2.0 and get rid of Translator 🤔

Sounds fair, didn’t know Translate still had a roadmap for deprecation. I’d leave the whole test suite until 2.0 and put this pull request on hold

@JanEbbing
Copy link
Member

Your PR definitely makes sense. I'll discuss in the team how soon we want to switch to 2.0 across this library and others due to the DeepLClient changes.

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.

2 participants