Updated whitelist for navwalker library#44
Merged
Merged
Conversation
** Why are these changes being introduced: * We previously excluded the third-party library Navwalker (included in the parent theme) from our code analysis tools. The library is also present in the child theme, however - so it must be excluded there as well. * The parent theme copy of Navwalker was also not quite completely whitelisted - it was ignored in the security-specific PHPCS file, but not rom the more general file that gets called by using PHPCS on the command line. ** Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/lm-142 ** How does this address that need: * This updates our CodeClimate and PHPCS configuration files to ignore both copies of navwalker. * It also moves the PHPCS exclusion from the security-specific file to the more general file. Now the library should be ignored in any context in which you might invoke PHPCS (at least, using the config files in this repo) ** Document any side effects to this change: * We will be more comprehensively ignoring whatever issues exist in the old version of Navwalker we use. There is a separate ticket in the backlog to convert this to a properly managed dependency.
Member
Author
|
A quick note - I see CodeClimate above flagging 2 issues, which confuses me. The files it is looking at don't change in this PR, so I'm not sure why there is an objection there. Certainly other issues exist in this repo, so ... why those two? Why only those two? 🤷 The GitHub Actions failures are down to content escaping problems, which will be resolved with the next PR in this process. |
JPrevost
approved these changes
Jan 12, 2023
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
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 extends our whitelisting of the Navwalker library in the parent theme to also include the copy of Navwalker in the child theme.
How we implement that whitelist in PHPCS is altered, moving the exclusion from the security-specific config to the more general config file.
We still have LM-162 in the backlog, which will try and convert this file to something managed by Composer. That would take both copies of the file out of this repo, and hopefully allow us to upgrade the version as well.
Relevant ticket(s):
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)