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

[1.10.x] Create DirectoryCreator helper class #2697

Merged
merged 3 commits into from
Nov 5, 2023

Conversation

calebdw
Copy link
Contributor

@calebdw calebdw commented Oct 26, 2023

Hello!

This is similar to #572 --- every other mkdir call is guarded by an is_dir check so I updated this to be consistent.

Thanks!

@phpstan-bot
Copy link
Collaborator

You've opened the pull request against the latest branch 1.11.x. If your code is relevant on 1.10.x and you want it to be released sooner, please rebase your pull request and change its target to 1.10.x.

@calebdw calebdw changed the base branch from 1.11.x to 1.10.x October 26, 2023 10:55
@ondrejmirtes
Copy link
Member

As far as PHP is concerned, this code is equivalent. You don't need to check is_dir before calling mkdir, the current code is fine.

I suspect the problem has a different root cause in Pest which should be addressed instead.

@calebdw
Copy link
Contributor Author

calebdw commented Oct 26, 2023

Sure, I get that---likely having to do with how it's capturing warnings. But there's still three other calls to mkdir that are already guarded by an is_dir check so I feel either they should all be guarded or none of them should be guarded for consistency

@ondrejmirtes
Copy link
Member

If we wanted to make this consistent everywhere, I'd welcome a new class DirectoryCreator with a static method that would be used in place of these calls.

@calebdw
Copy link
Contributor Author

calebdw commented Oct 30, 2023

@ondrejmirtes done---please let me know if you'd like any other changes

@calebdw calebdw force-pushed the mkdir branch 3 times, most recently from 5ac3465 to 0d8a040 Compare October 30, 2023 18:49
src/Internal/FilesystemHelper.php Outdated Show resolved Hide resolved
src/Cache/FileCacheStorage.php Outdated Show resolved Hide resolved
src/Internal/DirectoryCreator.php Outdated Show resolved Hide resolved
src/Internal/DirectoryCreator.php Outdated Show resolved Hide resolved
@calebdw calebdw changed the title [1.10.x] Check that dir does not exist before calling mkdir [1.10.x] Create DirectoryCreator helper class Nov 2, 2023
@calebdw calebdw force-pushed the mkdir branch 3 times, most recently from d223897 to 1bbe0f2 Compare November 2, 2023 19:32
@calebdw calebdw force-pushed the mkdir branch 2 times, most recently from 12dbf26 to 91d88e2 Compare November 2, 2023 20:08
Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

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

After this it's good to merge 👍

src/Internal/DirectoryCreator.php Show resolved Hide resolved
@ondrejmirtes ondrejmirtes merged commit 5980c07 into phpstan:1.10.x Nov 5, 2023
413 of 417 checks passed
@ondrejmirtes
Copy link
Member

Thank you.

@ondrejmirtes
Copy link
Member

Just did this to improve compatibility with PHPUnit and PHP-Parser 5: 9dac90d

@calebdw calebdw deleted the mkdir branch January 11, 2024 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants