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

Add File\leaky_temporary_file() #181

Closed
wants to merge 2 commits into from

Conversation

fredemmott
Copy link
Contributor

This is a temporary file that is not automatically deleted.

Name is following the "if it's sometimes necessary but you don't want
people to use it, name it something scary" principle :p

Adding this based on looking into why some open source Facebook projects
(e.g. fbshipit) use PHP IO instead of HSL IO.

File\temporary_file() should be strongly preferred over this, but
sometimes it's needed.

Test plan:

new unit test

Reviewers: atry, hsl, jonjanzen

This is a temporary file that is not automatically deleted.

Name is following the "if it's sometimes necessary but you don't want
people to use it, name it something scary" principle :p

Adding this based on looking into why some open source Facebook projects
(e.g. fbshipit) use PHP IO instead of HSL IO.

`File\temporary_file()` should be strongly preferred over this, but
sometimes it's needed.

Test plan:

new unit test

Reviewers: atry, hsl, jonjanzen
@facebook-github-bot
Copy link
Contributor

@fredemmott has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

*
* `File\temporary_file()` is **strongly** recommended instead.
*
* - If the prefix starts with `.`, it is interpretered relative to the current
Copy link
Member

Choose a reason for hiding this comment

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

"interpretered" is fun to say but not a word :P

expect(\file_exists($tf1->getPath()))->toBeTrue();
await $tf2->writeAllAsync('hello');
$tf2->close();
expect(\file_get_contents($tf2->getPath()))->toEqual('hello');
Copy link
Member

Choose a reason for hiding this comment

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

Maybe annotate this line to say that the file should be leaked here?

Otherwise it's kinda unclear what this is testing

@facebook-github-bot
Copy link
Contributor

@fredemmott has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot pushed a commit to facebook/hhvm that referenced this pull request Mar 15, 2022
Summary:
This is a temporary file that is not automatically deleted.

Name is following the "if it's sometimes necessary but you don't want
people to use it, name it something scary" principle :p

Adding this based on looking into why some open source Facebook projects
(e.g. fbshipit) use PHP IO instead of HSL IO.

`File\temporary_file()` should be strongly preferred over this, but
sometimes it's needed.

X-link: hhvm/hsl#181

Reviewed By: bigfootjon

Differential Revision: D34314199

fbshipit-source-id: 8513311c431861b2dfde9b8242702ef7f7585f5a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants