-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
refactor repo_stats to use os.pipe #11726
refactor repo_stats to use os.pipe #11726
Conversation
You've done the pipe bit correctly as far as I can see. The only thing is that you're throwing away the stderr which would be useful if there's an error so you need to keep it and merge it with the error when there's an error. Given we have to keep the authors names and email addresses in memory I worry that there is still a potential memory problem although this is a lot better. It may be that we have to run stats by periods and then progressively update otherwise the load on initial import could be bad. |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions. |
Signed-off-by: Andrew Thornton <art27@cantab.net>
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.
jup
credit to @zeripath for discovering inefficiency and proposing solution.