Skip to content

Activate intel runner for mac CI#364

Merged
asmacdo merged 4 commits intocon:mainfrom
CodyCBakerPhD:patch-2
Jan 9, 2026
Merged

Activate intel runner for mac CI#364
asmacdo merged 4 commits intocon:mainfrom
CodyCBakerPhD:patch-2

Conversation

@CodyCBakerPhD
Copy link
Copy Markdown
Contributor

@CodyCBakerPhD CodyCBakerPhD commented Jan 9, 2026 •
edited by asmacdo
Loading

Fixes #362

Fingers crossed

Copilot AI review requested due to automatic review settings January 9, 2026 16:27
@CodyCBakerPhD CodyCBakerPhD self-assigned this Jan 9, 2026
Copilot AI reviewed Jan 9, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request activates an Intel-based macOS runner in the CI workflow to complement the existing ARM64 (Apple Silicon) testing infrastructure.

Key changes:

  • Adds macos-15-intel runner to test both Intel and ARM64 macOS architectures
  • Enables comprehensive testing across different CPU architectures on macOS

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

@CodyCBakerPhD
remove warning
360ec2c 
Removed Intel architecture warning for macOS.
@asmacdo
Copy link
Copy Markdown
Member

asmacdo commented Jan 9, 2026

Instead of removing warnings we can filter them out https://github.com/con/duct/blob/main/tox.ini#L61-L64

Note there might be 2 warnings soon #363

@CodyCBakerPhD
Copy link
Copy Markdown
Contributor Author

CodyCBakerPhD commented Jan 9, 2026

I was hoping maybe there was no longer performance issues to warn about

But indeed seems flakier than arm64 still: https://github.com/con/duct/actions/runs/20858590518/job/59931985067?pr=364

@CodyCBakerPhD CodyCBakerPhD marked this pull request as draft January 9, 2026 16:45
@CodyCBakerPhD
Copy link
Copy Markdown
Contributor Author

CodyCBakerPhD commented Jan 9, 2026

Well, CI feels better - but I'm guessing this still means performance is degraded compared to arm/linux - do you still want to keep the warning in that case?

@asmacdo
Copy link
Copy Markdown
Member

asmacdo commented Jan 9, 2026

For the sigint failures, its startup time, which i dont think needs a warning. If it has problems collecting samples though, I think a warning would be nice. For now, I think lets drop the warning, and put it back in if necessary.

We can also drop strtobool so I'll close #363

@CodyCBakerPhD
Drop str2bool and warnings
0f17615 
Removed unused boolean conversion function and sets.
@CodyCBakerPhD CodyCBakerPhD marked this pull request as ready for review January 9, 2026 17:33
@CodyCBakerPhD CodyCBakerPhD added enhancement New feature or request semver-minor Increment the minor version when merged labels Jan 9, 2026
@CodyCBakerPhD CodyCBakerPhD requested a review from asmacdo January 9, 2026 17:33
@asmacdo asmacdo merged commit 2d8fcd7 into con:main Jan 9, 2026
27 of 28 checks passed
@CodyCBakerPhD CodyCBakerPhD deleted the patch-2 branch January 9, 2026 18:51
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 21, 2026

🚀 PR was released in v0.20.0 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@asmacdo asmacdo asmacdo approved these changes

Assignees

@CodyCBakerPhD CodyCBakerPhD

Labels

enhancement New feature or request released semver-minor Increment the minor version when merged

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Test Mac on intel

3 participants

@CodyCBakerPhD @asmacdo