Skip to content

Conversation

timazhum
Copy link
Contributor

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
Contributor 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
Contributor 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