-
Notifications
You must be signed in to change notification settings - Fork 79
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
imp: avoid unnecessary clones and validations when creating ClientId #1015
Conversation
Introduce ClientType::get_client_id method which constructs a new ClientId but forgoes client type verification. The client type has already been verified at creation and the verification guarantees that formatted ClientId will also be valid. Furthermore, rewrite ClientId::new so that it doesn’t take ownership of the client type and doesn’t allocated temporary strings. Furthermore, since ClinetType::get_client_id is now a thing, change the client type argument to be `&str` so that constructing ids from string client type is easier.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1015 +/- ##
==========================================
+ Coverage 70.90% 70.93% +0.03%
==========================================
Files 178 178
Lines 17995 17983 -12
==========================================
- Hits 12759 12756 -3
+ Misses 5236 5227 -9 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice optimization. Awesome 👌🏻
Just a few comments, we can merge then.
.changelog/unreleased/breaking-changes/1014-better-client-id-new.md
Outdated
Show resolved
Hide resolved
…ew.md Co-authored-by: Farhad Shabani <Farhad.Shabani@gmail.com> Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙏
…1015) * imp: avoid unnecessary clones and validations when creating ClientId Introduce ClientType::get_client_id method which constructs a new ClientId but forgoes client type verification. The client type has already been verified at creation and the verification guarantees that formatted ClientId will also be valid. Furthermore, rewrite ClientId::new so that it doesn’t take ownership of the client type and doesn’t allocated temporary strings. Furthermore, since ClinetType::get_client_id is now a thing, change the client type argument to be `&str` so that constructing ids from string client type is easier. * wip * clippy * Update .changelog/unreleased/breaking-changes/1014-better-client-id-new.md Co-authored-by: Farhad Shabani <Farhad.Shabani@gmail.com> Signed-off-by: Michal Nazarewicz <mina86@mina86.com> * build_client_id * drop variant * Update 1014-better-client-id-new.md Signed-off-by: Michal Nazarewicz <mina86@mina86.com> --------- Signed-off-by: Michal Nazarewicz <mina86@mina86.com> Co-authored-by: Farhad Shabani <Farhad.Shabani@gmail.com>
Introduce ClientType::get_client_id method which constructs a new
ClientId but forgoes client type verification. The client type has
already been verified at creation and the verification guarantees that
formatted ClientId will also be valid.
Furthermore, rewrite ClientId::new so that it doesn’t take ownership
of the client type and doesn’t allocated temporary strings.
Furthermore, since ClinetType::get_client_id is now a thing, change
the client type argument to be
&str
so that constructing ids fromstring client type is easier.
Closes: #1014
PR author checklist:
unclog
.docs/
).Reviewer checklist:
Files changed
in the GitHub PR explorer.