Skip to content

Comments

chore(services): unify scheme usage#6764

Merged
Xuanwo merged 1 commit intomainfrom
unify-scheme-usage
Nov 8, 2025
Merged

chore(services): unify scheme usage#6764
Xuanwo merged 1 commit intomainfrom
unify-scheme-usage

Conversation

@koushiro
Copy link
Member

@koushiro koushiro commented Nov 8, 2025

Which issue does this PR close?

Part of #6755

Rationale for this change

improve consistency

What changes are included in this PR?

  • Use string-based scheme specification over enum-based approach
  • remove Scheme enum usage from core services

Are there any user-facing changes?

@koushiro koushiro requested a review from Xuanwo as a code owner November 8, 2025 11:08
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. releases-note/chore The PR has a title that begins with "chore" or changes other small things that hard to tell labels Nov 8, 2025
@koushiro
Copy link
Member Author

koushiro commented Nov 8, 2025 •
edited
Loading

Perhaps we should consider removing the Scheme enum from the codebase later?
After merging this PR, the Scheme enum will only be used for the two public APIs:

  • Operator::via_iter: I think this API is more suitable for test cases than for production environments, and the existing code in use reflects this view.
  • Operator::via_map: had been deprecated, use via_iter instead. BTW, do we have a strategy for removing deprecated APIs? I haven't found any similar discussions yet.

Xuanwo
Xuanwo approved these changes Nov 8, 2025
View reviewed changes
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Thank you for working on this!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Nov 8, 2025
@Xuanwo Xuanwo merged commit 043977e into main Nov 8, 2025
328 checks passed
@Xuanwo Xuanwo deleted the unify-scheme-usage branch November 8, 2025 11:54
@Xuanwo
Copy link
Member

Xuanwo commented Nov 8, 2025

Perhaps we should consider removing the Scheme enum from the codebase later? After merging this PR, the Scheme enum will only be used for the two public APIs:

  • Operator::via_iter: I think this API is more suitable for test cases than for production environments, and the existing code in use reflects this view.
  • Operator::via_map: had been deprecated, use via_iter instead. BTW, do we have a strategy for removing deprecated APIs? I haven't found any similar discussions yet.

Yep, that's our goal to finally remove it. We can keep Scheme as an alias to &'static str to avoid breaking APIs to much.

@koushiro koushiro mentioned this pull request Nov 8, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Xuanwo Xuanwo Xuanwo approved these changes

Assignees

No one assigned

Labels

lgtm This PR has been approved by a maintainer releases-note/chore The PR has a title that begins with "chore" or changes other small things that hard to tell size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@koushiro @Xuanwo