News theme punch list fixes #82
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
** Why are these changes being introduced: * The news theme has a class that was meant to extend Walker_Nav_Menu, but is never actually implemented anywhere in the theme. The class itself is no longer available from the author's github, and an attempt to load it as-is indicates that it may not be compatible with current PHP versions because of argument mismatches with Walker_Nav_Menu. * The parent theme already implements its own extension to the Walker_Nav_Menu class (navwalker.php), which _does_ work with modern PHP. The News theme thus does not need its own class. ** Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/lm-310 ** How does this address that need: * This removes the unneeded and unsupported class from the News theme. ** Document any side effects to this change: * If I've missed a reference to this somewhere, something will break.
** Why are these changes being introduced: * Category displays like /news/category/archives-mit-history/ should include content from all post types on the site (all of which can be placed into categories), rather than just show Post content. ** Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/lm-309 ** How does this address that need: * This adds a function from the legacy News site which expands the category display to include all post types (as long as the category in question isn't a bibliotech issue - those should still be scoped only to Bibliotech issues). ** Document any side effects to this change: * Part of this commit is to update the function name and comment in functions.php, which was not accurate in the legacy theme - leading me to initially leave this function out of the theme.
JPrevost
approved these changes
Apr 10, 2023
matt-bernhardt
pushed a commit
that referenced
this pull request
May 10, 2023
* create a new function to check jq if jq doesn't exist, look for brew or apt-get. if neither of those exist, exit hard. * update the command * [BUGS-6035] Adding upstream .lando file for installation of 'jq', 'npm', etc. (#83) 73:Adding upstream .lando file for installation of 'jq', 'npm', etc. * add link to jq downloads page * require shellcheck this might mean we need to tweak some of the gh actions to ensure shellcheck is installed there, we'll see if this fails... * update the composer scripts & add the lint script * suggest installing jq manually * ignore shellcheck unused vars for colors * shellcheck linting suggestiosn * actually lint * comment out color/style variables we aren't using * replace == with -eq * switch back to == because it's required * switch -eq back to == * use fewer conditionals bail early and often if we've met our conditions Co-authored-by: Phil Tyler <philip@tylerdigital.com> --------- Co-authored-by: Charles Leverington <charles.leverington@gmail.com> Co-authored-by: Phil Tyler <philip@tylerdigital.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adjusts two aspects of the News site based on the internal review i conducted after the content import:
Developer
Secrets
Documentation
Accessibility
our guide and
all issues introduced by these changes have been resolved or opened as new
issues (link to those issues in the Pull Request details above)
Stakeholder approval
Dependencies
NO dependencies are updated
Code Reviewer
(not just this pull request message)