Skip to content
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

Trial: Improve memory handling ... #1435

Merged
merged 2 commits into from Dec 11, 2014
Merged

Conversation

benallard
Copy link
Contributor

No description provided.

@sa2ajj
Copy link
Contributor

sa2ajj commented Dec 9, 2014

👍

stream, line = yield

def createSummary(self, loog):
problems = self.problems
problems = '\n'.join(self.problems)
Copy link
Contributor

Choose a reason for hiding this comment

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

I forgot to make one comment: the resulting problems will not have trailing \n. If it's intended, I'll merge it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Logs are not intended to be machine parse able. Hence I think it's no problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking about the change that was introduced in #1436

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sa2ajj: Because of that trailing '\n', that bug was discovered. But in no way, a trailing '\n' is needed.

sa2ajj pushed a commit that referenced this pull request Dec 11, 2014
Trial: Improve memory handling ...
@sa2ajj sa2ajj merged commit cfab602 into buildbot:master Dec 11, 2014
@benallard benallard deleted the patch-1 branch December 12, 2014 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants