Reland "Cleanup tools/emprofile.py and add more testing. NFC (#13403)" #13416
Conversation
|
The updated test seems to pass fine on mac and windows here... without any changed :-/ Do you have any links to the failing builds? Any idea why they failed? |
sbc100
force-pushed
the
reland
branch
5 times, most recently
from
a409584 to
68c4178
Compare
Run the new test on windows to prove that it works there. This was reverted in 13410
|
View with "ignore whitespace" to better see what changed. |
| return files | ||
| except IOError: | ||
| return [] | ||
| files.append(f) |
There was a problem hiding this comment.
Somewhat indifferent about changing files += [f] to files.append(f), though is there a specific reason to favor append instead of the += form? iiuc the array += [element] syntax has been extensively used in the past in this repo?
There was a problem hiding this comment.
Yeah I don't feel strongly about it.
I think its a good habit to get into when building lists in python because it avoids creating the extra lists each time and (sometimes importantly) avoids re-binding the variable each time. (When you do += you end up throwing away both the old list and the single element list on the RHS. Not that is performance not relevant in this particular case :)
|
lgtm, nice cleanups! |
Run the new test on windows to prove that it works there.
This was reverted in #13410