- 
                Notifications
    
You must be signed in to change notification settings  - Fork 13.9k
 
Implement padding for IpAddr without heap alloc #67035
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
Conversation
| 
           Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @shepmaster (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information.  | 
    
1db21c6    to
    757014f      
    Compare
  
    757014f    to
    f89a0b5      
    Compare
  
    | 
           The job  Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact   | 
    
f89a0b5    to
    64fee0a      
    Compare
  
    64fee0a    to
    fbf91e5      
    Compare
  
    | 
           The job  Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact   | 
    
fbf91e5    to
    7e711fc      
    Compare
  
    | 
           @shepmaster Any thoughts on this?  | 
    
| 
           shepmaster appears to be busy, I'd suggest someone else @rust-lang/libs review this. To me personally, the implementation looks good, but I don't know if I can r+ this (I've only recently been added to the org and no one explained to me how this works yet).  | 
    
| 
           Let's pick someone else from libs, indeed... RNG says: r? @SimonSapin  | 
    
| 
           FYI please don't ping @SimonSapin unless they are specifically the best person to review. #66341 (comment) r? @dtolnay  | 
    
| 
           LGTM once there is a test.  | 
    
7e711fc    to
    5852426      
    Compare
  
    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.
Looks good. Thanks!
| 
           @bors r+  | 
    
| 
           📌 Commit 58524262c2258d821a5f3b48ce6b5a9e5813e08f has been approved by   | 
    
| 
           Make sure your work email is listed in https://github.com/settings/emails if you want this commit attributed to your GitHub account.  | 
    
| 
           @dtolnay Thanks for the heads up, I didn't even realize this was a thing  | 
    
5852426    to
    a34a96f      
    Compare
  
    | 
           Thanks @dtolnay. @shepmaster for finding reviewers, may I suggest drawing from https://github.com/rust-lang/highfive/blob/master/highfive/configs/rust-lang/rust.json ?  | 
    
          
 Thanks. I tend to just look at the teams page; perhaps it's silly of me to assume they are identical, but the web page is way easier to find.  | 
    
| 
           ☔ The latest upstream changes (presumably #67540) made this pull request unmergeable. Please resolve the merge conflicts.  | 
    
a34a96f    to
    32d6a45      
    Compare
  
    32d6a45    to
    915686d      
    Compare
  
    | 
           @shepmaster I’ve opened rust-lang/highfive#241 for a potential way to make that easier.  | 
    
| 
           @dtolnay I'm not sure if it actually makes a difference for bors, but I think this should be   | 
    
| 
           @bors r+  | 
    
| 
           📌 Commit 915686d has been approved by   | 
    
| 
           ☀️ Test successful - checks-azure  | 
    
Implements padding for
IpAddrs without heap allocations.This fixes issue #66810 .
cc @jethrogb @mzohreva