Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IBX-6045: Introduced attributes argument to Generator::startValueElement signature #65

Merged
merged 4 commits into from
Jun 28, 2023

Conversation

alongosz
Copy link
Member

@alongosz alongosz commented Jun 27, 2023

Question Answer
JIRA issue IBX-6045
Required by ibexa/shipping#25
Type bug
Target Ibexa version v4.6.0
BC breaks yes

This PR adds missing 3rd argument $attributes to \Ibexa\Contracts\Rest\Output\Generator::startValueElement contract signature. The existing implementations

  • \Ibexa\Rest\Output\Generator\Json::startValueElement
  • \Ibexa\Rest\Output\Generator\Xml::startValueElement

have been relying on this argument for the past 11 years, so it's safe to declare this as a bug.

Documentation

Technically it's a BC break which requires documenting in 4.6.0 upgrade instructions and release notes.

The method signature of \Ibexa\Contracts\Rest\Output\Generator::startValueElement got improved and now looks as follows:

    /**
     * @phpstan-param scalar $value
     * @phpstan-param array<string, scalar> $attributes
     */
    abstract public function startValueElement(string $name, $value, array $attributes = []): void;

any 3rd party code extending \Ibexa\Contracts\Rest\Output\Generator needs to update the signature.

Checklist:

  • Provided PR description.
  • Tested the solution manually.
  • Provided automated test coverage.
  • Checked that target branch is set correctly.
  • Ran PHP CS Fixer for new PHP code (use $ composer fix-cs).
  • Asked for a review.

@alongosz alongosz added Bug Something isn't working Doc needed The changes require some documentation Ready for review labels Jun 27, 2023
@alongosz alongosz requested review from Steveb-p, ViniTou and a team June 27, 2023 11:51
@konradoboza konradoboza requested a review from a team June 27, 2023 11:53
alongosz and others added 2 commits June 27, 2023 15:54
@alongosz alongosz force-pushed the ibx-6045-fix-startValueElement-signature branch from 167e192 to e5cedf7 Compare June 27, 2023 14:01
@sonarcloud
Copy link

sonarcloud bot commented Jun 27, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@alongosz alongosz requested a review from Steveb-p June 27, 2023 14:06
@alongosz
Copy link
Member Author

QA: there's nothing to test here really besides what CI REST functional/integration tests did. It's a change on abstract which has been already present in the implementations for the last 11 years and affects static analysis only (and 3rd party extending it - unlikely scenario).

@alongosz alongosz merged commit cb19434 into main Jun 28, 2023
@alongosz alongosz deleted the ibx-6045-fix-startValueElement-signature branch June 28, 2023 09:24
@MagdalenaZuba MagdalenaZuba removed the Doc needed The changes require some documentation label Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants