Skip to content

Conversation

@m4tx
Copy link
Member

@m4tx m4tx commented Nov 24, 2025

No description provided.

Copilot AI review requested due to automatic review settings November 24, 2025 16:49
Copy link

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.

Copilot wasn't able to review any files in this pull request.


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

@github-actions
Copy link

github-actions bot commented Nov 24, 2025

🐰 Bencher Report

Branchbump-lockfile
Testbedgithub-ubuntu-latest
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
microseconds (µs)
(Result Δ%)
Upper Boundary
microseconds (µs)
(Limit %)
empty_router/empty_router📈 view plot
🚷 view threshold
6,179.10 µs
(+1.87%)Baseline: 6,065.71 µs
6,750.32 µs
(91.54%)
json_api/json_api📈 view plot
🚷 view threshold
1,048.90 µs
(+2.15%)Baseline: 1,026.82 µs
1,127.00 µs
(93.07%)
nested_routers/nested_routers📈 view plot
🚷 view threshold
954.09 µs
(+0.22%)Baseline: 952.00 µs
1,046.60 µs
(91.16%)
single_root_route/single_root_route📈 view plot
🚷 view threshold
922.70 µs
(+1.45%)Baseline: 909.51 µs
990.61 µs
(93.15%)
single_root_route_burst/single_root_route_burst📈 view plot
🚷 view threshold
16,926.00 µs
(-4.08%)Baseline: 17,646.15 µs
21,010.44 µs
(80.56%)
🐰 View full continuous benchmarking report in Bencher

@m4tx m4tx changed the title chore: bump deps chore: bump deps, use MSRV-aware deps resolver Nov 24, 2025
Copilot AI review requested due to automatic review settings November 24, 2025 17:57
@github-actions github-actions bot added the A-ci Area: CI (Continuous Integration) label Nov 24, 2025
Copy link

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

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.


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

strategy:
matrix:
rust: [stable, nightly, "1.86"] # 1.85 is the MSRV
rust: [stable, "1.86"] # 1.85 is the MSRV
Copy link

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

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

The comment states "1.85 is the MSRV" but the code tests version "1.86" which matches the rust-version = "1.86" in Cargo.toml. The comment should be updated to say "1.86 is the MSRV" to avoid confusion.

Suggested change
rust: [stable, "1.86"] # 1.85 is the MSRV
rust: [stable, "1.86"] # 1.86 is the MSRV

Copilot uses AI. Check for mistakes.
@m4tx m4tx force-pushed the bump-lockfile branch 2 times, most recently from e0f4f88 to 6034093 Compare November 24, 2025 18:18
Copilot AI review requested due to automatic review settings November 24, 2025 18:18
Copy link

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

Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.


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

Comment on lines 60 to 61
- rust: nightly
continue-on-error: true
Copy link

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

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

The nightly build configuration is incomplete. Adding nightly only to the include section without an os value means it will only run on one OS (the default), but this is not explicitly defined. To run nightly builds on all platforms, you should either:

  1. Add it back to the matrix and handle errors differently, or
  2. Specify the os in the include section explicitly

Suggested fix:

include:
  - rust: nightly
    os: ubuntu-latest
    continue-on-error: true
  - rust: nightly
    os: macos-latest
    continue-on-error: true
  - rust: nightly
    os: windows-latest
    continue-on-error: true
Suggested change
- rust: nightly
continue-on-error: true
- rust: nightly
os: ubuntu-latest
continue-on-error: true
- rust: nightly
os: macos-latest
continue-on-error: true
- rust: nightly
os: windows-latest
continue-on-error: true

Copilot uses AI. Check for mistakes.

name: Build & test
runs-on: ${{ matrix.os }}
continue-on-error: ${{ matrix.continue-on-error }}
Copy link

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

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

The continue-on-error property will be undefined for most matrix combinations. The expression ${{ matrix.continue-on-error }} will evaluate to an empty string (falsy) for stable and 1.86 builds, which is the desired behavior. However, for the nightly build(s), this will work correctly.

A more explicit approach would be:

continue-on-error: ${{ matrix.continue-on-error || false }}

This makes the default behavior explicit.

Suggested change
continue-on-error: ${{ matrix.continue-on-error }}
continue-on-error: ${{ matrix.continue-on-error || false }}

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings November 24, 2025 18:33
Copy link

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

Copilot reviewed 2 out of 3 changed files in this pull request and generated no new comments.


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

@m4tx m4tx changed the title chore: bump deps, use MSRV-aware deps resolver chore: use MSRV-aware resolver, ignore nightly failures Nov 24, 2025
@m4tx m4tx merged commit c8f76b5 into master Nov 24, 2025
41 of 43 checks passed
@m4tx m4tx deleted the bump-lockfile branch November 24, 2025 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ci Area: CI (Continuous Integration)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants