Re: cvs: php-src(PHP_5_3) /ext/standard basic_functions.c basic_functions.h /ext/standard/tests/general_functions ini_get_all.phpt
| From: | Jani Taskinen | Date: | Mon, 14 Apr 2008 19:15:42 +0000 |
| Subject: | Re: cvs: php-src(PHP_5_3) /ext/standard basic_functions.c basic_functions.h /ext/standard/tests/general_functions ini_get_all.phpt | ||
| References: | 1 2 3 4 5 | Groups: | php.internals |
| Request: | Send a blank email to internals+get-37007@lists.php.net to get a copy of this message | ||
If you can't read, here's again: "It was originally only enabled for debug builds and should always still be, I've no idea why Marcus made it enabled always in the first place."
That should answer all your questions. And no, it's still not doing anything similar like ini_get_all().
Now, be a nice boy and revert and study the code more before you commit anything else.
For the rest of you peeps: You should actually start reviewing the patches and not just say "it's ok, commit" in IRC..
--Jani
Hannes Magnusson kirjoitti:
On Mon, Apr 14, 2008 at 7:43 PM, Jani Taskinen <jani.taskinen@sci.fi> wrote:Hannes Magnusson kirjoitti:Rule#1: Test the stuff you write, not only in your mind, with .phpt too Rule#2: Make sure there are not more than 3 functions doing almost the exact same thing. Rule#3: Subscribe to internals@ and participate in the discussion of your work Rule#4: Add comments about any hidden meanings behind the function Rule#5: Add a frckin [DOC] note to the commit explaining how the stuff works so it can be documented Rule#6: If you write a function just to so it can be used when debugging another internal feature add it in #ifdef FEATURE_DEBUG or whatever so other people don't come after you and wonder wtf you were thinking Rule#7: If a documentor asks for clarifications on internals@ then help him Rule#8: Do not reclassify bug-reports as documentation problem without further information If you don't tell anyone about what you are doing you cannot expect anyone else to know what your intentions are. If the function should be documented, removed, changed, only in debug mode, only there or only here. Marcus apparently thought you made a booboo and renamed it and enabled it in normal builds, and I seeing absolutely no point in yet another ini-get function combined it with another function seemingly serving the same purpose, in slightly different format... I did ask on internals@ if we really did need yet-another-ini-get function and since noone replied I followed your basic principle: Commit and let the trolls flame. -Hannes (learned from the best! ;))On Mon, Apr 14, 2008 at 5:40 PM, Jani Taskinen <jani.taskinen@sci.fi>wrote:against.Eh..do you really understand the code correctly..? ini_get_all() is very much different what config_dump_hash() was/is.Uhh.. I don't see the difference. And there are no tests to compare itI already asked on internals@ and after no reply I tracked down helly on IRC and he was fine with merging them.This function was originally just meant for debugging the config hash. And hint for the difference is: registered vs. anything (that was freebie..) Rule 1: Learn how to use CVS before you commit (hint: cvs annotate filename.ext) Rule 2: Once you learned how to use CVS and figured out who to ask: ASK. Rule 3: Ask the right people before committing. Rule 4: IRC != internals@ (I haven't seen ANY question about this on internals!) Now be good boy and revert the crap, that function is VERY useful for debugging how the php.ini stuff works (or not) and should be there. It was originally only enabled for debug builds and should always still be, I've no idea why Marcus made it enabled always in the first place..