Skip to content

refactor: use more modern cfg_select! macro#3

Merged
mgerhold merged 1 commit into
mainfrom
use-cfg-select-macro
Jun 18, 2026
Merged

refactor: use more modern cfg_select! macro#3
mgerhold merged 1 commit into
mainfrom
use-cfg-select-macro

Conversation

@mgerhold

Copy link
Copy Markdown
Collaborator

No description provided.

@mgerhold mgerhold force-pushed the use-cfg-select-macro branch 3 times, most recently from c22886d to 92db41a Compare June 18, 2026 09:59
@mgerhold mgerhold force-pushed the use-cfg-select-macro branch from 92db41a to a6a0180 Compare June 18, 2026 10:06
@mgerhold mgerhold marked this pull request as ready for review June 18, 2026 10:13
@mgerhold mgerhold requested a review from Copilot June 18, 2026 10:13

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the cross-platform test command helpers to use a single-function implementation per helper, selecting OS-specific behavior via the cfg_select! macro instead of duplicating functions behind item-level #[cfg(...)] guards.

Changes:

  • Replaced paired #[cfg(windows)] / #[cfg(not(windows))] function definitions with cfg_select! { windows => { ... } _ => { ... } } inside a single function body across multiple helpers.
  • Kept item-level #[cfg] for cat_cmd specifically, documenting why (per-branch constness differs).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sn0opy sn0opy left a comment

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.

looks good 👍

@mgerhold mgerhold merged commit 8ac0dd7 into main Jun 18, 2026
14 of 15 checks passed
@mgerhold mgerhold deleted the use-cfg-select-macro branch June 18, 2026 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants