Skip to content

Conversation

@darckking
Copy link

Signed-off-by: illia [email protected]

Q A
Documentation no
Bugfix no
BC Break no
New Feature yes
RFC no
QA no

This is a solution for issue #18 I posted last year. As there was no response so far I decided to create a pull request.

Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

This kind of change requires test additions for validation and regression prevention

@Ocramius Ocramius added Awaiting Author Updates Bug Something isn't working labels Jan 7, 2022
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

LGTM. Need someone else's review too, to see if this is the valid approach, but it is OK from my PoV.

@Ocramius
Copy link
Member

Also to be clarified: do we want this in 1.10.x, or 1.11.x (next minor)?

Copy link
Contributor

@weierophinney weierophinney left a comment

Choose a reason for hiding this comment

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

This makes total sense to me.

Yes, it's a bugfix, but also a behavior change, so I'd argue we leave it for new minor.

@weierophinney weierophinney merged commit b0969bd into laminas-api-tools:1.11.x Jan 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants