Skip to content

Conversation

@ahmedsbytes
Copy link

@ahmedsbytes ahmedsbytes commented Jan 28, 2019

Bug #51068

Fix DirectoryIterator glob://* current path relative queries

@KalleZ
Copy link
Member

KalleZ commented Jan 28, 2019

Hi,

Not sure what went wrong with the other PR anyway.

The path and path_len should be declared at the top of the block else I think the PR looks good

@ahmedsbytes
Copy link
Author

@KalleZ Moved hopefully to the right place :)

@KalleZ
Copy link
Member

KalleZ commented Jan 28, 2019

Thanks @amaabdou, lets see what the CI runs reports

@ahmedsbytes
Copy link
Author

I tried to check AppVeyor output but could not understand if my changes is related to
"Build execution time has reached the maximum allowed time for your plan (60 minutes)."
is this related to my changes ?

@KalleZ
Copy link
Member

KalleZ commented Jan 28, 2019

@amaabdou, actually as long as Travis passes I think we should be good, I reckon I saw a comment by @nikic the other day regarding the exact same issue with the timeout of AppVeyor and that we should just ignore it if Travis passes.

@ahmedsbytes
Copy link
Author

Thanks @KalleZ
Is there anything else here I should do for this PR ?

@krakjoe
Copy link
Member

krakjoe commented Jan 29, 2019

Can you update target branch please.

@ahmedsbytes ahmedsbytes changed the base branch from master to PHP-7.2 January 29, 2019 07:24
Fix DirectoryIterator glob://* current path relative queries
@ahmedsbytes
Copy link
Author

I'm not sure which target branch should be since the bug exists in all 7.x but I assumed 7.2 is the right one and re-based against it, if not please tell me which one :)

@petk
Copy link
Member

petk commented Jan 29, 2019

PHP 7.1 is for security related patches so yes, PHP 7.2 is the correct one here...

@petk petk added the Bug label Jan 29, 2019
@nikic
Copy link
Member

nikic commented Feb 11, 2019

Sorry for the delay, now merged as ec28d4c into 7.2+. Thanks!

@nikic nikic closed this Feb 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants