Skip to content

Conversation

@ptoffy
Copy link
Member

@ptoffy ptoffy commented Sep 23, 2025

No description provided.

ptoffy and others added 4 commits September 1, 2025 12:10
* Fix benchmarks take two

* Add throughput thresholds

* Wip
* First attempt at Swift 6 router

* Make router immutable in benchmarks

* Update benchmarks manifest

* Re-add swift settings

* Make options immutable

* Update thresholds

* Update thresholds

* Update benchmarks

* Adjust thresholds

* Adjust thresholds

* Use appropriate property

* Create RouterBuilder

* Update benchmarks

* Address review and sprinke some `@inlinable`

* Add back package setting

* Update benchmarks

* Update benchmark thresholds

* Adjust threshold
* Implement partial matching

* Attempt at making it fast

* Cleanup

* Undo

* Partial Matching 2 (#143)

* Different approach to partial matching

* Nits

* Avoid a copy

* Minor improvements

* Nit

* Update Sources/RoutingKit/PathComponent.swift

Co-authored-by: Gwynne Raskind <[email protected]>

* Update Sources/RoutingKit/TrieRouter/TrieRouterNode.swift

Co-authored-by: Gwynne Raskind <[email protected]>

* Nits

* Nit

---------

Co-authored-by: Gwynne Raskind <[email protected]>
@ptoffy ptoffy marked this pull request as ready for review October 27, 2025 11:19
@ptoffy ptoffy requested review from 0xTim and gwynne as code owners October 27, 2025 11:19
@codecov
Copy link

codecov bot commented Oct 27, 2025

Codecov Report

❌ Patch coverage is 87.55981% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.33%. Comparing base (1a10cce) to head (a3190b8).

Files with missing lines Patch % Lines
Sources/RoutingKit/TrieRouter/TrieRouterNode.swift 76.00% 12 Missing ⚠️
...rces/RoutingKit/TrieRouter/TrieRouterBuilder.swift 83.33% 11 Missing ⚠️
Sources/RoutingKit/TrieRouter/TrieRouter.swift 95.58% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #141      +/-   ##
==========================================
+ Coverage   83.91%   88.33%   +4.42%     
==========================================
  Files           4        5       +1     
  Lines         230      240      +10     
==========================================
+ Hits          193      212      +19     
+ Misses         37       28       -9     
Files with missing lines Coverage Δ
Sources/RoutingKit/Parameters.swift 88.23% <ø> (ø)
Sources/RoutingKit/PathComponent.swift 100.00% <100.00%> (ø)
Sources/RoutingKit/TrieRouter/TrieRouter.swift 95.58% <95.58%> (ø)
...rces/RoutingKit/TrieRouter/TrieRouterBuilder.swift 83.33% <83.33%> (ø)
Sources/RoutingKit/TrieRouter/TrieRouterNode.swift 76.00% <76.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@0xTim 0xTim left a comment

Choose a reason for hiding this comment

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

Some comments

@usableFromInline typealias Node = TrieRouterNode<Output>

/// Available ``TrieRouter`` customization options.
public enum ConfigurationOption: Sendable {
Copy link
Member

Choose a reason for hiding this comment

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

Should we have a full configuration type instead? That would allow us to make this a Bool so we can avoid the enum for future configuration options

Copy link
Member

Choose a reason for hiding this comment

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

How much has this changed from the original router?


@Test func testPartial() throws {
var routerBuilder = TrieRouterBuilder<Int>()
routerBuilder.register(42, at: ["test", ":{my-file}.json"])
Copy link
Member

Choose a reason for hiding this comment

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

Let's add some more tests to test things like a partial parameter only at the start, only at the end, failures for unclosed parameters etc

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.

2 participants