Skip to content

Conversation

@oliverklee
Copy link
Collaborator

Part of #756

@coveralls
Copy link

coveralls commented Feb 27, 2025

Coverage Status

coverage: 54.655%. remained the same
when pulling c70a770 on cleanup/hungarian/rules
into 58ef076 on main.

@oliverklee oliverklee force-pushed the cleanup/hungarian/rules branch from 06a14f2 to c70a770 Compare February 27, 2025 18:55
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.

The change is fine. I'm struggling to understand the original implementation.

Comment on lines +274 to +275
foreach ($this->rules as $rules) {
foreach ($rules as $rule) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm struggling to understand how and why this can be doubly-iterated over - since Rule does not appear to be Iterable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don’t understand this either. Either this is broken (and not tested, or never called), or the code somehow still is working fine. 😉

@JakeQZ JakeQZ merged commit c22d738 into main Feb 27, 2025
21 checks passed
@JakeQZ JakeQZ deleted the cleanup/hungarian/rules branch February 27, 2025 19:24
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