Skip to content

[mssql] Remove redundant all_logs conditional branch#4268

Merged
arif-ali merged 1 commit intososreport:mainfrom
reidliu41:cleanup/mssql-remove-redundant-branch
Mar 10, 2026
Merged

[mssql] Remove redundant all_logs conditional branch#4268
arif-ali merged 1 commit intososreport:mainfrom
reidliu41:cleanup/mssql-remove-redundant-branch

Conversation

@reidliu41
Copy link
Contributor

The if/else block at lines 81-86 was dead code: both branches
performed identical add_copy_spec() calls, and the same paths
were already collected unconditionally at lines 75-79.

This redundancy dates back to commit 967c015, which moved
sizelimit handling into the framework. The original branches
had differed only in their sizelimit arguments, but after that
refactor the two branches became identical, leaving a no-op
conditional behind.

While the framework deduplicates copy_paths (stored as a set),
removing this block improves readability and avoids confusion
about the intended role of the all_logs option in this plugin.


Please place an 'X' inside each '[]' to confirm you adhere to our Contributor Guidelines

  • Is the commit message split over multiple lines and hard-wrapped at 72 characters?
  • Is the subject and message clear and concise?
  • Does the subject start with [plugin_name] if submitting a plugin patch or a [section_name] if part of the core sosreport code?
  • Does the commit contain a Signed-off-by: First Lastname email@example.com?
  • Are any related Issues or existing PRs properly referenced via a Closes (Issue) or Resolved (PR) line?
  • Are all passwords or private data gathered by this PR obfuscated?

  The if/else block at lines 81-86 was dead code: both branches
  performed identical add_copy_spec() calls, and the same paths
  were already collected unconditionally at lines 75-79.

  This redundancy dates back to commit 967c015, which moved
  sizelimit handling into the framework. The original branches
  had differed only in their sizelimit arguments, but after that
  refactor the two branches became identical, leaving a no-op
  conditional behind.

  While the framework deduplicates copy_paths (stored as a set),
  removing this block improves readability and avoids confusion
  about the intended role of the all_logs option in this plugin.

Signed-off-by: reidliu41 <reid201711@gmail.com>
@packit-as-a-service
Copy link

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

  • sudo dnf install -y 'dnf*-command(copr)'
  • dnf copr enable packit/sosreport-sos-4268
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

Copy link
Contributor

@pmoravec pmoravec left a comment

Choose a reason for hiding this comment

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

Nice catch.

@arif-ali arif-ali added the Reviewed/Ready for Merge Has been reviewed, ready for merge label Mar 10, 2026
@arif-ali arif-ali merged commit adb275c into sosreport:main Mar 10, 2026
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Reviewed/Ready for Merge Has been reviewed, ready for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants