Hi! I want the code to be clearer about which values are from configuration, which are from user input, and which are just local variables. Using $GLOBALS on it's own doesn't really address that concern. You could use something like $GLOBALS['conf']['ganglia_dir'], but it seems unnecessarily long.
We could make a convention like "$GLOBALS is for configuration", which I think is the approach you've taken with your changes in conf.php so far. My only objection to that is that I'd also like to set up something like $user to hold all the values which come from user input. Again, that could be in $GLOBALS['user'], but I'd favor just a regular array. It's shorter & more readable. Most of what's currently going on in ganglia is in the top-level scope, so there isn't a lot of practical difference between $GLOBALS['conf']['ganglia_dir'] and $conf['ganglia_dir'], though I think the readability is better for $conf. Inside functions, I'd like to see argument passing instead of globals, but that's really a separate issue. (Passing $conf as an argument is also makes functions easier to test, and I do want to see us get some automated testing coverage on the functions we do have.) Thoughts? alex On Feb 25, 2011, at 10:36 AM, Vladimir Vuksan wrote: > > Alex, > > I have been using the $GLOBALS array for global configuration options. I > would recommend using that instead of $conf > > http://sourceforge.net/apps/trac/ganglia/browser/branches/monitor-web-2.0/conf.php.in?rev=2489#L8 > > Thanks, > Vladimir > > On Fri, 25 Feb 2011 09:51:17 -0600, Alex Dean <a...@crackpot.org> wrote: >> I created >> http://bugzilla.ganglia.info/cgi-bin/bugzilla/show_bug.cgi?id=299. > Added >> the conf-conversion script & patches for eval_config.php and graph.php. >> I'll continue to convert other scripts as time allows. >> >> alex >> >> On Feb 24, 2011, at 10:05 PM, Alex Dean wrote: >> >>> I wrote a script to read in a conf.php file and convert it to use a >>> $conf array. I don't think we have a place for utility scripts like > this >>> right now. Where should it go? >>> https://github.com/alexdean/ganglia-stuff/blob/master/reformat_conf.php >>> >>> After you use this tool, but before all code is updated to use $conf, > we >>> can maintain backwards-compatibility with existing code with something >>> like this: >>> https://gist.github.com/843349 >>> >>> I've taken a quick tour through my local ganglia using a converted >>> conf.php and not seen any obvious problems, but of course the > non-obvious >>> ones are probably there somewhere. >>> >>> alex >>> >>> On Feb 24, 2011, at 7:01 AM, Jesse Becker wrote: >>> >>>> +1 to that too >>>> >>>> On Wed, Feb 23, 2011 at 23:49, Bernard Li <bern...@vanhpc.org> wrote: >>>>> +1 from me as well. >>>>> >>>>> I guess we should probably check it into both monitor-web-2.0 and >>>>> trunk. >>>>> >>>>> Cheers, >>>>> >>>>> Bernard >>>>> >>>>> On Wed, Feb 23, 2011 at 7:35 PM, Jesse Becker <haw...@gmail.com> > wrote: >>>>>> +1 >>>>>> >>>>>> On Wed, Feb 23, 2011 at 21:27, Alex Dean <a...@crackpot.org> wrote: >>>>>>> One of my gripes with the current PHP frontend code is how hard it >>>>>>> can be to recall where which variables are configuration, which > come >>>>>>> from user input, and which are just local variables. As one step >>>>>>> toward fixing this issue, I think it would be nice to place all >>>>>>> configuration values (mainly in conf.php currently) into a $conf >>>>>>> array. The benefit is that it's immediately clear in any code > which >>>>>>> uses these values that you're dealing with configuration values. >>>>>>> There's no danger of name collisions with other variables. >>>>>>> >>>>>>> I'm just wondering if others feel the same way, and would support a >>>>>>> change like this. It's pretty straighforward to do, but would >>>>>>> obviously touch a lot of different code. Before I go ahead with >>>>>>> making all those changes, I guess I'd just like to know if there > are >>>>>>> any huge objections out there to this idea. Take a look, let me > know >>>>>>> what you think. I'd like to do something similar for user input as >>>>>>> well, maybe $user? >>>>>>> >>>>>>> http://pastie.org/1600587 >>>>>>> >>>>>>> thanks, >>>>>>> alex >>>>>>> >>>>>>> >>>>>>> >>>>>>> > ------------------------------------------------------------------------------ >>>>>>> Free Software Download: Index, Search & Analyze Logs and other IT >>>>>>> data in >>>>>>> Real-Time with Splunk. Collect, index and harness all the fast >>>>>>> moving IT data >>>>>>> generated by your applications, servers and devices whether >>>>>>> physical, virtual >>>>>>> or in the cloud. Deliver compliance at lower cost and gain new >>>>>>> business >>>>>>> insights. http://p.sf.net/sfu/splunk-dev2dev >>>>>>> _______________________________________________ >>>>>>> Ganglia-developers mailing list >>>>>>> Ganglia-developers@lists.sourceforge.net >>>>>>> https://lists.sourceforge.net/lists/listinfo/ganglia-developers >>>>>>> >>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Jesse Becker >>>>>> >>>>>> > ------------------------------------------------------------------------------ >>>>>> Free Software Download: Index, Search & Analyze Logs and other IT >>>>>> data in >>>>>> Real-Time with Splunk. Collect, index and harness all the fast > moving >>>>>> IT data >>>>>> generated by your applications, servers and devices whether > physical, >>>>>> virtual >>>>>> or in the cloud. Deliver compliance at lower cost and gain new >>>>>> business >>>>>> insights. http://p.sf.net/sfu/splunk-dev2dev >>>>>> _______________________________________________ >>>>>> Ganglia-developers mailing list >>>>>> Ganglia-developers@lists.sourceforge.net >>>>>> https://lists.sourceforge.net/lists/listinfo/ganglia-developers >>>>>> >>>>> >>>> >>>> >>>> >>>> -- >>>> Jesse Becker >>>> >>> >>> >>> > ------------------------------------------------------------------------------ >>> Free Software Download: Index, Search & Analyze Logs and other IT data >>> in >>> Real-Time with Splunk. Collect, index and harness all the fast moving > IT >>> data >>> generated by your applications, servers and devices whether physical, >>> virtual >>> or in the cloud. Deliver compliance at lower cost and gain new business > >>> insights. http://p.sf.net/sfu/splunk-dev2dev >>> _______________________________________________ >>> Ganglia-developers mailing list >>> Ganglia-developers@lists.sourceforge.net >>> https://lists.sourceforge.net/lists/listinfo/ganglia-developers >>> >> >> >> > ------------------------------------------------------------------------------ >> Free Software Download: Index, Search & Analyze Logs and other IT data > in >> Real-Time with Splunk. Collect, index and harness all the fast moving IT >> data >> generated by your applications, servers and devices whether physical, >> virtual >> or in the cloud. Deliver compliance at lower cost and gain new business >> insights. http://p.sf.net/sfu/splunk-dev2dev >> _______________________________________________ >> Ganglia-developers mailing list >> Ganglia-developers@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/ganglia-developers > ------------------------------------------------------------------------------ Free Software Download: Index, Search & Analyze Logs and other IT data in Real-Time with Splunk. Collect, index and harness all the fast moving IT data generated by your applications, servers and devices whether physical, virtual or in the cloud. Deliver compliance at lower cost and gain new business insights. http://p.sf.net/sfu/splunk-dev2dev _______________________________________________ Ganglia-developers mailing list Ganglia-developers@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ganglia-developers
