-
Notifications
You must be signed in to change notification settings - Fork 6
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
DEP Remove remaining cwp dependencies and code #55
DEP Remove remaining cwp dependencies and code #55
Conversation
a4a4842
to
85678c0
Compare
85678c0
to
c0fa387
Compare
app/_config/environmentcheck.yml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was here in part as a way to validate that environmentcheck is working in kitchen sink.
Please keep the top config (which doesn't actually depend on anything CWP related) and just rename it to not say "cwp".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated, though have removed the cwp-search bit since there's no CMS 6 module for that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good, that's what I intended by "keep the top config" - "do not keep the bottom config" was implied. 👍
app/_config/theme.yml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please just replace watea
and starter
with simple
but otherwise keep this, unless an identical theme.yml
is provided by one of kitchen sink's dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
composer.json
Outdated
"project-files": [ | ||
"app/*" | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to keep this if we're keeping some of the above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
c0fa387
to
ab16e9b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Issue silverstripe/.github#199
Requires silverstripe/silverstripe-versionfeed#101 to fix the recipe-contents blocks jobs, and will need to be merged up as that PR is targetting 4.13 and this is a 6 build