Uploaded image for project: 'eZ Publish / Platform'
  1. eZ Publish / Platform
  2. EZP-22562

Create missing value object visitors services

    Details

      Description

      A couple ValueObjectVisitor classes haven't been mapped to services, as they're part of the REST/Client namespace.

      Commit @d39b764e can be taken as an example, and even cherry-picked in the pull-request's branch.

      Note that an extra commit, https://github.com/ezsystems/ezpublish-kernel/commit/0be433c5f4febbc0dd15f1205faa66736c965f16, is currently required to avoid a scope error. I'm investigating a separate fix for it. In the meanwhile, the commit can also be cherry-picked.

        Activity

        Hide
        Bertrand Dunogier added a comment -

        Required by QA-306.

        Show
        Bertrand Dunogier added a comment - Required by QA-306.
        Hide
        Marcos Loureiro (Inactive) added a comment -

        About this, is there any kind of testing need to be done, or is it already tested?
        ( i was thinking in doing "manual" testing by commands to see expected results )
        ping
        Bertrand Dunogier
        André Rømcke
        Petar Spanja

        Show
        Marcos Loureiro (Inactive) added a comment - About this, is there any kind of testing need to be done, or is it already tested? ( i was thinking in doing "manual" testing by commands to see expected results ) ping Bertrand Dunogier André Rømcke Petar Spanja
        Hide
        Bertrand Dunogier added a comment -

        The visitors themselves should already be tested: https://github.com/ezsystems/ezpublish-kernel/tree/master/eZ/Publish/Core/REST/Client/Tests/Output/ValueObjectVisitor. Their integration as Symfony2 services isn't for the server. We could do it, but I don't see the point of unit testing this part as long as we do using functional tests.

        Do you agree, guys ?

        But any fix/change to the visitors will require that the unit tests for those are updated accordingly.

        Show
        Bertrand Dunogier added a comment - The visitors themselves should already be tested: https://github.com/ezsystems/ezpublish-kernel/tree/master/eZ/Publish/Core/REST/Client/Tests/Output/ValueObjectVisitor . Their integration as Symfony2 services isn't for the server. We could do it, but I don't see the point of unit testing this part as long as we do using functional tests . Do you agree, guys ? But any fix/change to the visitors will require that the unit tests for those are updated accordingly.
        Hide
        Marcos Loureiro (Inactive) added a comment -

        But any fix/change to the visitors will require that the unit tests for those are updated accordingly.

        Got it.

        When you can, or André Rømcke please push this to Doc done.

        Show
        Marcos Loureiro (Inactive) added a comment - But any fix/change to the visitors will require that the unit tests for those are updated accordingly. Got it. When you can, or André Rømcke please push this to Doc done.
        Hide
        Marcos Loureiro (Inactive) added a comment - - edited

        All PR's

        ContentTypeGroup
        ContentTypeGroup: https://github.com/ezsystems/ezpublish-kernel/pull/795
        ContentTypeGroupInput: https://github.com/ezsystems/ezpublish-kernel/pull/795

        Complete PR: https://github.com/ezsystems/ezpublish-kernel/pull/795

        Other details needed on the PR description

        Show
        Marcos Loureiro (Inactive) added a comment - - edited All PR's ContentTypeGroup ContentTypeGroup: https://github.com/ezsystems/ezpublish-kernel/pull/795 ContentTypeGroupInput: https://github.com/ezsystems/ezpublish-kernel/pull/795 Complete PR: https://github.com/ezsystems/ezpublish-kernel/pull/795 Other details needed on the PR description
        Hide
        Bertrand Dunogier added a comment -

        It won't work like this, you can't base all of your pull-requests on the same branch that isn't master... we need to merge my commits first, if you do it like this.

        Show
        Bertrand Dunogier added a comment - It won't work like this, you can't base all of your pull-requests on the same branch that isn't master... we need to merge my commits first, if you do it like this.
        Show
        Marcos Loureiro (Inactive) added a comment - Merged: https://github.com/ezsystems/ezpublish-kernel/commit/8da87ead7a6a81b1a9b32b39cb361129fe7ca1f4

          People

          • Assignee:
            Unassigned
            Reporter:
            Bertrand Dunogier
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - Not Specified
              Not Specified
              Remaining:
              Remaining Estimate - 0 minutes
              0m
              Logged:
              Time Spent - 1 day, 3 hours
              1d 3h