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

FEATURE: Site configuration presets #4735

Merged
merged 5 commits into from Apr 30, 2024
Merged

Conversation

bwaidelich
Copy link
Member

Resolves: #4582

@mhsdesign
Copy link
Member

This looks already great and seems to be non breaking. Are there any open points left?

@bwaidelich
Copy link
Member Author

Are there any open points left?

I urgently need to document open tasks when I mark a PR as draft.. I don't know what I planned to do still – maybe adjust docs on docs.neos.io or add tests? 🙈

I'll mark it ready for review now

@bwaidelich bwaidelich marked this pull request as ready for review November 28, 2023 12:07
@mhsdesign
Copy link
Member

mhsdesign commented Jan 13, 2024

Okay as far as i can tell is the change non breaking as the wildcard * pattern still works - though one is probably better of declaring the config for a siteNode specifically and using a preset?

~So the neos ui test dist and neos demo doesnt need to be migrated? https://github.com/neos/neos-ui/blob/5758aa4d935c9fa06dcf55a0e3b55fca3961b89e/Tests/IntegrationTests/TestDistribution/DistributionPackages/Neos.Test.OneDimension/Configuration/Settings.yaml#L35~  Exactly this change is totally non breaking.

The documentation at https://docs.neos.io/guide/manual/site-configuration would need to be updated as well please.

Copy link
Member

@mhsdesign mhsdesign left a comment

Choose a reason for hiding this comment

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

This feature is needed for my masterplan in #4470 (comment)
so i will gladly approve this.

My previous concerns, to remove support for * and document on docs.neos.io will be part of my followup. Also that will keep this pr from being breaking.

@bwaidelich
Copy link
Member Author

Shall we merge this?

@bwaidelich
Copy link
Member Author

I rebased this one and would like to merge it if tests are green.
@mhsdesign are you OK with that?
I'll take care of the docs immediately afterwards

@bwaidelich bwaidelich mentioned this pull request Apr 26, 2024
Copy link
Member

@mhsdesign mhsdesign left a comment

Choose a reason for hiding this comment

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

Yeah sure, lets do it. (Even-though the logic really grows in the SiteConfiguration thingy and i might like to have a unit test for it 🙈)

@bwaidelich
Copy link
Member Author

i might like to have a unit test for it

yes, good point! I might like to have one as well :D I'll take care

@mhsdesign
Copy link
Member

❤️ +2 ;)

@bwaidelich bwaidelich merged commit 0b46cbc into 9.0 Apr 30, 2024
10 checks passed
Copy link
Contributor

@dlubitz dlubitz left a comment

Choose a reason for hiding this comment

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

Looks good. Thank you!

@bwaidelich bwaidelich deleted the feature/4582-site-config-presets branch April 30, 2024 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Site Config Presets
3 participants