Repository navigation
Conversation
Contributor
|
First, thanks for this PR... Could you separate the the ini directive fix and the constant fix to independent PR? Each of them should be taken care of individually. The rest looks great, except on phpt which I think has the "debuging" typo as part of the test. Also, please rebase your patch so we won't have all those merged with upstream commits, merge --ff-only is your friend (: |
This was referenced Jul 14, 2013
Closed
Contributor
Author
Contributor
|
We shouldn't be changing constant names, that breaks BC. Yes, I know some names may be wrong, but it's better to have them this way than break builds for everybody. |
|
Comment on behalf of stas at php.net: split into three pulls |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mostly not serious things.
But there is 2 more interesting:
2e15d1f HASH_KEY_NON_EXISTANT fix
0d67a40 mssql.compatability_mode fix
The first seems to be used in lots of places. The second one touches configuration argument... Probably It is better to keep it untouched. Anyway the patch is ready, the decision is yours.
I think the other modifications are not really painful.