Skip to content

Conversation

@rohan-varma
Copy link
Contributor

@rohan-varma rohan-varma commented Mar 25, 2021

Stack from ghstack:

Some operator<< code manually implemented string join in C++, turns
out there is a c10 util for this. Use the util instead of rolling our own.

Differential Revision: D27316705

Some operator<< code manually implemented string join in C++, turns
out there is a c10 util for this. Use the util instead of rolling our own.

Differential Revision: [D27316705](https://our.internmc.facebook.com/intern/diff/D27316705/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Mar 25, 2021

💊 CI failures summary and remediations

As of commit 801145c (more details on the Dr. CI page):


💚 💚 Looks good so far! There are no failures yet. 💚 💚


This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

rohan-varma added a commit that referenced this pull request Mar 25, 2021
Some operator<< code manually implemented string join in C++, turns
out there is a c10 util for this. Use the util instead of rolling our own.

Differential Revision: [D27316705](https://our.internmc.facebook.com/intern/diff/D27316705/)

ghstack-source-id: 124840043
Pull Request resolved: #54649
Copy link
Contributor

@wayi1 wayi1 left a comment

Choose a reason for hiding this comment

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

Thanks for the refactoring!

I remember last time I suggested using the string join function in folly, but I was told that folly is not available in torch codebase. Glad that we still have quite some util functions.

@rohan-varma
Copy link
Contributor Author

Thanks for the refactoring!

I remember last time I suggested using the string join function in folly, but I was told that folly is not available in torch codebase. Glad that we still have quite some util functions.

Yeah, looks like there are quite a lot of utils in c10/ dir that I was previously unaware of - I should probably check there before implementing common things one off in the future.

@codecov
Copy link

codecov bot commented Mar 25, 2021

Codecov Report

Merging #54649 (801145c) into gh/rohan-varma/268/base (c371542) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

@@                     Coverage Diff                     @@
##           gh/rohan-varma/268/base   #54649      +/-   ##
===========================================================
- Coverage                    77.47%   77.47%   -0.01%     
===========================================================
  Files                         1893     1893              
  Lines                       185802   185795       -7     
===========================================================
- Hits                        143948   143941       -7     
  Misses                       41854    41854              

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 6b7652e.

@facebook-github-bot facebook-github-bot deleted the gh/rohan-varma/268/head branch March 29, 2021 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants