Skip to content

Conversation

@oliverklee
Copy link
Collaborator

Part of #756

@coveralls
Copy link

coveralls commented Feb 24, 2025

Coverage Status

coverage: 53.778%. remained the same
when pulling 734366b on cleanup/hungarian/parser-state
into e725c2e on main.

Copy link
Collaborator

@JakeQZ JakeQZ left a comment

Choose a reason for hiding this comment

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

Could we improve on the naming of the $amount parameter, as we are renaming it?


/**
* @param int $iAmount
* @param int $amount
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could possibly improve this parameter name while renaming it. Perhaps $characterCount or $numberOfCharacters.

/**
* @param string $string
* @param string $sNeedle
* @param string $haystack
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good rename to match the PHP docs.

@oliverklee oliverklee force-pushed the cleanup/hungarian/parser-state branch from 38fd14b to 734366b Compare February 24, 2025 13:43
@oliverklee oliverklee requested a review from JakeQZ February 24, 2025 13:43
@JakeQZ JakeQZ merged commit a3e4b93 into main Feb 24, 2025
21 checks passed
@JakeQZ JakeQZ deleted the cleanup/hungarian/parser-state branch February 24, 2025 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants