-
Notifications
You must be signed in to change notification settings - Fork 3
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
Four Kitchens Release - Sprint 41 #1425
Conversation
* fix(shs-5118): add to calendar theming * fix(shs-5118): removing stray border * fix(shs-5118): fixing theming for traditional version * fix(shs-5118): a11y updates to addtocal button
…ons (#1429) * feat(shs-4998): create uniform height checkbox field * feat(shs-4998): create deploy hook to update existing hs_collection paragraphs * feat(shs-4998): Update uniform height conditional field settings and visibility * feat(shs-4998): add check to uniform height that both uniform height and raised cards is enabled
* fix(shs-5178): ocean theme contrast fixes * fix(shs-5178): a few more contrast fixes * fix(shs-5178): limiting changes to colorful theme * chore(shs-5178): Update compiled css files --------- Co-authored-by: Andrés Díaz Soto <[email protected]> Co-authored-by: Andrés Díaz Soto <[email protected]>
Co-authored-by: Andrés Díaz Soto <[email protected]>
@@ -0,0 +1,29 @@ | |||
<?php |
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.
What is this file? i've never heard of a deploy.php file nor do I understand how the function is intended to be used. I don't find any documentation for deploy hooks.
If this is just a simple update hook, move it to an install or post_update file
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.
@pookmish Deploy hooks are part of drush and are executed after the configuration is imported. There's more information here:
- https://www.drush.org/12.x/deploycommand/
- https://www.hashbangcode.com/article/drupal-9-different-update-hooks-and-when-use-them
We consider is the best way for cases like this, when the db updates depend on configuration that's exported in the same PR. However, if you consider that's better to use an update hook, please let us know and we can change it.
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.
It looks like deploy hooks are only part of drush and not part of core. There is an issue in the queue to add them to core, but there's not a lot of traction on it. It seems really useful but I'm a little concerned it's only drush supported.
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.
I agree with Joe. To add to that, we would have to modify the deployment process to add the drush command. Let's move it to an update/post_update hook.
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.
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.
Just need to swap for a service.
*/ | ||
function hs_paragraph_types_update_10000() { | ||
$config_directories = Settings::get('config_sync_directory'); | ||
$file_storage = new FileStorage($config_directories); |
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.
There is a service for this: config.storage
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.
@pookmish Fixed.
READY FOR REVIEW
Summary
Need Review By (Date)
01/15
Urgency
medium
Steps to Test
SHS-5118
SHS-5180
a. Anonymous
b. Authenticated
c. Contributor
d. Author
e. Site Manager
SHS-4998
Add Component
buttonRaised Cards
checkbox causes theUniform Height
checkbox to appear, already enabled.Items Per Row
to 2Uniform Height
checkboxAdd Component
button in the Collection component, add 2 Postcard componentshb-raised-cards
classEdit
button on the Collection componentUniform Height
checkboxhb-raised-cards
classRaised Cards
enabled should haveUniform Height
enabled as well. The following were used to test during development:1.
Understanding Attitudes in the West
(Raised Cards enabled)2.
Student Profiles
(Raised Cards disabled)1.
Department External Reviews
(Raised Cards disabled)2.
Events
(Raised Cards enabled)SHS-5178
https://archaeology.suhumsci.loc/academics/field-experience
https://history.suhumsci.loc/people/caroline-winterer
https://archaeology.suhumsci.loc/events/distinguished-lecture-series/gathering-shadows-material-things
See "SUNet Login" in the footer area on any page
https://history.suhumsci.loc/events/upcoming-events
https://history.suhumsci.loc/recent-news
https://history.suhumsci.loc/events/upcoming-events
SHS-5179
https://southasia.suhumsci.loc/admin/config/media/image-styles/manage/postcard_vertical_linked_465px_
Before:
PR Checklist