Skip to content

Conversation

@ahmedsbytes
Copy link

Fix DirectoryIterator glob://* current path relative queries

@php-pulls php-pulls merged commit a9a02ca into php:master Jan 28, 2019
@ahmedsbytes
Copy link
Author

Not sure why "pull bot" closed the MR , reopened it here
#3771

@petk
Copy link
Member

petk commented Jan 28, 2019

Bugs in php-pulls helper bot? Interesting...

@nikic
Copy link
Member

nikic commented Jan 28, 2019

You likely pushed master to this PR, so it effectively ended up as a no-op PR that is considered "merged".

@ahmedsbytes
Copy link
Author

@nikic should I always create a branch ? And not use the matser ?

@petk
Copy link
Member

petk commented Jan 30, 2019

@amaabdou yes. Sending pull request from the master branch is never good actually. Also because you'll have issues updating your master branch on your repository when the php master branch gets updated. For example, current master branch in php-src is already the PHP-8 branch in development actually.

@petk
Copy link
Member

petk commented Jan 30, 2019

A good practice:

git clone git@github.com:your-username/php-src
cd php-src
git checkout -b patch-1
git add .
git commit -m "Describe changes"
git push origin patch-1

A good practice is to also set the upstream remote in case the upstream master branch updates. This way your master branch will track remote upstream master branch of the root repository.

git checkout master
git remote add upstream git://github.com/php/php-src
git config branch.master.remote upstream
git pull --rebase

For another example, to send a pull request against the PHP upstream PHP-7.2 branch:

cd php-src
git checkout -b bug-patch upstream/PHP-7.2
git add .
git commit -m "Describe changes"
git push origin bug-patch

and open pull request to target the PHP-7.2 branch in php-src.

@ahmedsbytes
Copy link
Author

Thanks @petk for the advices, with all the mess I did to my repo I see what you mean clearly
But cool, Ive learned for my next PR :)

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.

6 participants