Opened 3 years ago

Closed 3 years ago

#24754 closed enhancement (implemented)

Remove unnecessary heap allocations in Rust protover implementation.

Reported by: frewsxcv Owned by:
Priority: Medium Milestone: Tor: 0.3.3.x-final
Component: Core Tor/Tor Version:
Severity: Minor Keywords: review-group-28
Cc: coreyf@… Actual Points:
Parent ID: Points:
Reviewer: Sponsor:


This is my first Tor patch! 🎉

I was working towards and noticed we do some unnecessary heap allocations via collect.

Even though this is a small change I'm mostly opening to see what the review/merge process is like. Also curious if small patches like this should be folded into larger patches as separate commits, or if it's okay to open individual tickets for them. If there's anything I'm doing wrong, let me know – I've done virtually no open source work outside of GitHub and am new to this Trac-based workflow.

Child Tickets

Change History (5)

comment:1 Changed 3 years ago by frewsxcv

Status: newneeds_review

comment:2 Changed 3 years ago by nickm

Milestone: Tor: 0.3.3.x-final

Welcome! Thanks for the patch, and sorry for the delay in reviewing: we've mostly been on vacation last week.

This patch looks good to me, but I'm a rust newb -- can another rust person have a look before we merge?

comment:3 Changed 3 years ago by nickm

Keywords: review-group-28 added

comment:4 Changed 3 years ago by chelseakomlo

This looks good to me as well, using an iterator is a nice improvement.

comment:5 Changed 3 years ago by nickm

Resolution: implemented
Status: needs_reviewclosed

Thanks for the review, Chelsea!

Merging this branch to master.

Note: See TracTickets for help on using tickets.