Skip to content

Conversation

MarcoPNS
Copy link
Contributor

@MarcoPNS MarcoPNS commented Jan 8, 2021

Just updated my fork to Neos 7.0 (the one with the resolutionHost feature #3) - It may be helpful to apply the changes here as well.

Copy link

@bwaidelich bwaidelich left a comment

Choose a reason for hiding this comment

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

I can't test this right now, but thanks a lot for putting so much effort into this It looks great by reading!

I just left a nitpick comment for now

@MarcoPNS
Copy link
Contributor Author

MarcoPNS commented Jan 8, 2021

@bwaidelich Thank you for your feedback! Just renamed it.

Copy link

@bwaidelich bwaidelich left a comment

Choose a reason for hiding this comment

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

did you rename the files, too? since it doesn't seem so but I might got confused by the github ui

@MarcoPNS
Copy link
Contributor Author

The file has also been renamed. What still needs to be rewritten are the tests. I have not yet updated them.

* Test case for the BackendUriDimensionPresetDetector
*/
class DetectContentSubgraphComponentTest extends FunctionalTestCase
class DetectContentSubgraphMiddlewareTest extends FunctionalTestCase

Choose a reason for hiding this comment

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

File needs to be renamed accordingly

@bwaidelich
Copy link

The file has also been renamed

ah right. It's just the test file that hasn't been renamed it seems

@putzwasser
Copy link

putzwasser commented Feb 13, 2021

I updated to 7.0 and it broke neos-dimensionresolver. Found this PR. Is this PR ready for merging? :)

@MarcoPNS
Copy link
Contributor Author

just checked if everything is renamed and it should be save to merge.

@gitvitox
Copy link

Any updates on this? @bwaidelich

Copy link

@bwaidelich bwaidelich left a comment

Choose a reason for hiding this comment

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

no time to test atm, but this looks good by reading. Thanks!

Copy link
Contributor

@albe albe left a comment

Choose a reason for hiding this comment

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

Looks okay as far as I can tell.

@albe
Copy link
Contributor

albe commented Mar 30, 2021

Let's merge 🤞

@albe albe merged commit b3f8e4f into Flowpack:master Mar 30, 2021
@putzwasser
Copy link

Could you please tag this commit, so composer will install it without whining about minimum stability requirements :D ?

@MarcoPNS MarcoPNS deleted the only-neos-7-update branch July 13, 2022 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants