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

fix: close file in the generateAtDir func #3480

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

testwill
Copy link

@testwill testwill commented Apr 7, 2024

  • PR Description
    close file in the generateAtDir func
  • Please check if the PR fulfills these requirements
  • Cheatsheets are up-to-date (run go generate ./...)
  • Code has been formatted (see here)
  • Tests have been added/updated (see here for the integration test guide)
  • Text is internationalised (see here)
  • Docs (specifically docs/Config.md) have been updated if necessary
  • You've read through your own file changes for silly mistakes etc

@stefanhaller
Copy link
Collaborator

The change looks obviously right, but I'm curious what your reason was for making this change. Did you actually have a problem that is fixed by this? (Like, were you running out of file handles? Seems unlikely to me.) I find it helpful to mention this kind of information in the PR description and/or commit message.

@jesseduffield
Copy link
Owner

I also am simultaneously grateful for the PR and confused at the motivation. @testwill I see that you've got at least 600 repositories forked and most of them, from what I've seen, are forked for the purpose of raising a PR that fixes a typo or closes a file handle.

I'm curious: did you raise this PR yourself or was this automated?

@testwill
Copy link
Author

raise this PR myself. typo can use misspell, but not very precise. No tool can detect closing file handle,according to my understanding.

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