Skip to content

feature / normalize-rank-feature#57

Merged
tomas862 merged 1 commit intomainfrom
feature/normalize-rank-feature
Apr 7, 2026
Merged

feature / normalize-rank-feature#57
tomas862 merged 1 commit intomainfrom
feature/normalize-rank-feature

Conversation

@tomas862
Copy link
Copy Markdown
Member

@tomas862 tomas862 commented Apr 7, 2026

No description provided.

@tomas862 tomas862 merged commit 7add19b into main Apr 7, 2026
3 checks passed
@tomas862 tomas862 deleted the feature/normalize-rank-feature branch April 7, 2026 09:49
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a mode parameter to NormalizeRequest and a taskId to NormalizeFieldResult to support different normalization modes and asynchronous processing. The review feedback recommends adding validation for the mode parameter to ensure it only accepts 'linear' or 'rank' values, improving the robustness of the SDK.

public array $fields,
public ?string $mode = null,
) {
$this->validateFields($fields);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The mode parameter is currently not validated. To improve robustness and prevent sending invalid values to the API, it's best to validate that $mode is either null, 'linear', or 'rank', as specified in the PHPDoc. This provides early feedback to the developer using the SDK.

        if ($mode !== null && !in_array($mode, ['linear', 'rank'], true)) {
            throw new InvalidArgumentException(
                'Invalid normalization mode. Allowed values are "linear" or "rank".',
                'mode',
                $mode
            );
        }

        $this->validateFields($fields);

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.

1 participant