Skip to content

DeepL support, PHP8.4 support.#12

Open
adiwidjaja wants to merge 14 commits into
wernerkrauss:mainfrom
atwx:main
Open

DeepL support, PHP8.4 support.#12
adiwidjaja wants to merge 14 commits into
wernerkrauss:mainfrom
atwx:main

Conversation

@adiwidjaja
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Owner

@wernerkrauss wernerkrauss left a comment

Choose a reason for hiding this comment

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

Hi @adiwidjaja, thanks for the PR. some things are unclear to me and updates to tests and README is missing.

Comment thread _config/fluent-export-import.yml Outdated
# Available Backends are:
# class: Netwerkstatt\FluentExIm\Translator\ChatGPTTranslator

class: Netwerkstatt\FluentExIm\Translator\DeepLTranslator No newline at end of file
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

this makes Deepl the default. do we really want this for the module?

public function __construct(string $apiKey = null)
{
if($apiKey == null) {
$apiKey = $this->getAPIKey("CHATGPT_API_KEY");
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

this would be a good usage for a constant, wouldn't it?

public function __construct(string $apiKey = null)
{
if($apiKey == null) {
$apiKey = $this->getAPIKey("DEEPL_API_KEY");
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

see above. A constant like CLASS::API_KEY would be cleaner; this could als be used in the Trait, so no need to pass the constant as a string.

wernerkrauss and others added 6 commits January 7, 2026 13:40
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