Skip to content

fix resoruce leaks in tests and REPL, fixes #8358 #9036

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

Merged
merged 2 commits into from
May 26, 2020

Conversation

rethab
Copy link
Contributor

@rethab rethab commented May 25, 2020

No description provided.

Copy link
Member

@dottybot dottybot left a comment

Choose a reason for hiding this comment

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

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Have an awesome day! ☀️

Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM

s"""Each file has to start with the prompt: "$prompt"""")

def extractInputs(prompt: String): List[String] = {
def extractInputs(lines: BufferedIterator[String]): List[String] = {
Copy link
Member

Choose a reason for hiding this comment

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

Could this use a plain collection like a Seq instead of an Iterator? I find iterators way too easy to misuse (e.g., if I try to print their content, then I break any code afterward), and this code isn't performance critical.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I'll improve this 👍

@rethab rethab requested a review from smarter May 25, 2020 12:36
@rethab
Copy link
Contributor Author

rethab commented May 26, 2020

@smarter this is ready to merge I think

@smarter smarter merged commit 30bebc3 into scala:master May 26, 2020
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.

3 participants