Add `--exclude` argument for verifying checksums by ethicaladitya · Pull Request #124 · wp-cli/checksum-command · GitHub
Skip to content

Add --exclude argument for verifying checksums#124

Closed
ethicaladitya wants to merge 1 commit into
wp-cli:mainfrom
ethicaladitya:gh-64-add-exclude-argument
Closed

Add --exclude argument for verifying checksums#124
ethicaladitya wants to merge 1 commit into
wp-cli:mainfrom
ethicaladitya:gh-64-add-exclude-argument

Conversation

@ethicaladitya

Copy link
Copy Markdown

Added argument "--excluded" to the wp core verify-checksums to rule out any false positives when we have extra files in the core.

Related:
#64
wp-cli/wp-cli#5955

@ethicaladitya ethicaladitya requested a review from a team as a code owner June 13, 2024 14:48
@mrsdizzie

mrsdizzie commented Jun 13, 2024

Copy link
Copy Markdown
Member

@swissspidy swissspidy changed the title gh-64 added exclude argument in checksums Add --exclude argument for verifying checksums Aug 7, 2024
@swissspidy swissspidy added command:core-verify-checksums Related to 'core verify-checksums' command command:plugin-verify-checksums Related to 'plugin verify-checksums' command and removed command:plugin-verify-checksums Related to 'plugin verify-checksums' command labels Aug 7, 2024
*/
private function is_excluded( $file ) {
foreach ( $this->exclude_paths as $exclude_path ) {
if ( strpos( $file, $exclude_path ) !== false ) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is too broad IMHO. It means I can do --exclude=a and every file with the letter a in it will be excluded. I don't think that's desired.

We should make this a strict comparison, where the path has to match exactly. Could be a simple in_array() check.

"""
And STDERR should be empty

Scenario: Verify core checksums with excluded file

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

An additional test with a file in a subfolder would be beneficial, to verify that works too.

@swissspidy

Copy link
Copy Markdown
Member

@swissspidy swissspidy closed this Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

command:core-verify-checksums Related to 'core verify-checksums' command

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants