Skip to content

Rename BoxedRound::new_dynamic to just new #112

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dvdplm
Copy link
Collaborator

@dvdplm dvdplm commented May 26, 2025

Given that there are no other constructors available on BoxedRound I think it's better to name it just new.

@coveralls
Copy link

coveralls commented May 26, 2025

Pull Request Test Coverage Report for Build 15269611379

Details

  • 8 of 8 (100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.7%) to 72.966%

Totals Coverage Status
Change from base Build 15169064028: 0.7%
Covered Lines: 2170
Relevant Lines: 2974

💛 - Coveralls

@fjarri
Copy link
Member

fjarri commented May 26, 2025

Yeah, perhaps. It was a preparation for #65 which I'm not sure is coming

@dvdplm dvdplm force-pushed the dp-rename-new_dynamic branch from a1e55d1 to 2b8c678 Compare May 27, 2025 07:49
@dvdplm dvdplm requested a review from Copilot May 27, 2025 07:56
Copy link

@Copilot 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 PR renames the BoxedRound::new_dynamic constructor to the shorter BoxedRound::new, updating its definition, all call sites, and the changelog.

  • Renamed the constructor in boxed_round.rs
  • Updated every new_dynamic call to new across tests, benches, session code, combinators, and examples
  • Added a changelog entry for this API change

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
manul/src/tests/partial_echo.rs Updated call from new_dynamic to new
manul/src/session/session.rs Updated call from new_dynamic to new
manul/src/protocol/boxed_round.rs Renamed new_dynamic method to new
manul/src/combinators/misbehave.rs Updated calls from new_dynamic to new
manul/src/combinators/chain.rs Updated calls from new_dynamic to new
manul/benches/empty_rounds.rs Updated calls from new_dynamic to new
manul/benches/async_session.rs Updated calls from new_dynamic to new
examples/src/simple.rs Updated calls from new_dynamic to new
CHANGELOG.md Added entry for new_dynamicnew rename

@fjarri
Copy link
Member

fjarri commented Jun 7, 2025

If we decide to go with #117, it will invalidate this PR.

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