FUDforum
Fast Uncompromising Discussions. FUDforum will get your users talking.

Home » FUDforum Development » Plugins and Code Hacks » Install.php suggestion
Show: Today's Messages :: Polls :: Message Navigator
Switch to threaded view of this topic Create a new topic Submit Reply
Install.php suggestion [message #165402] Sat, 11 June 2011 17:43 Go to next message
Dayo is currently offline  Dayo   
Messages: 101
Registered: April 2011
Karma: 0
Senior Member
Attached is a diff of changes to install.php.

Two general types:

1. Grammar stuff (Minor): fixed some typos and tightened up some of the slightly rambling style
2. Variable Structure (Major): I found it really difficult to understand the logic and flow until I realised that when I see, say, "$WWW_ROOT" it may not refer to the "$WWW_ROOT" from GLOBALS.PHP. So I decided to go through and rationalise things by applying some structure.

Variables in all capital text now always originate from GLOBALS.PHP (mostly - apart from in function decompress_archive) and others are in lower case.

The "$WWW_ROOT" posted from the local form for instance has now been changed to "$form_www_root" and if there is any "$WWW_ROOT", then it has come from GLOBALS.PHP. This gives structure and now someone not as intimately familiar with the code as you devs can figure out what is what.

This obviously doesn't change anything functionally but sets the stage for a plan to latter look into restructuring GLOBALS.PHP to address my gripes as set out HERE.

This needs a proper review before it can be committed. I know that as it does not have a direct functional pay off, it may be seen as a PITA but sorting this will allow further stuff later that will help streamline the application.

Cheers.

****
Edit: Did a review myself and made a few changes. Revision attached.


[Updated on: Sun, 12 June 2011 00:37]

Report message to a moderator

Re: Install.php suggestion [message #165403 is a reply to message #165402] Sun, 12 June 2011 04:40 Go to previous messageGo to next message
naudefj is currently offline  naudefj   South Africa
Messages: 3771
Registered: December 2004
Karma: 28
Senior Member
Administrator
Core Developer
You may go ahead with 1: Grammar stuff (Minor).

However, I would vote against the variable changes. It is obvious that $_POST['WWW_ROOT'] is from the form, and that $WWW_ROOT will make its way into GLOBALS,php. On the other hand, it may be just because I've got so used to it.
Re: Install.php suggestion [message #165404 is a reply to message #165402] Sun, 12 June 2011 07:13 Go to previous messageGo to next message
Dayo is currently offline  Dayo   Bahrain
Messages: 101
Registered: April 2011
Karma: 0
Senior Member
Yes it is obvious the post var is from the form but only until it gets read into $WWW_ROOT at which point it isn't clear any longer. Perhaps you are too close to the issue to notice.

When I look at the proposed code, I can readily follow the structure through (I actually started making the changes in order to make sense of the file).

Anyway, your vote decides although I will be interested in knowing what disadvantages there are in applying what appears to me to be a clear structure as I assume your "no" vote is because of some issue with this approach you have identified or is it just a reluctance to change what "works" ... perhaps given the workload?

Just curious.
Re: Install.php suggestion [message #165405 is a reply to message #165404] Mon, 13 June 2011 04:36 Go to previous messageGo to next message
naudefj is currently offline  naudefj   South Africa
Messages: 3771
Registered: December 2004
Karma: 28
Senior Member
Administrator
Core Developer
Can you please split the two patches and commit the first?
We can revisit the variable changes after the 3.0.3 release.
Re: Install.php suggestion [message #165407 is a reply to message #165402] Mon, 13 June 2011 13:27 Go to previous messageGo to next message
Dayo is currently offline  Dayo   
Messages: 101
Registered: April 2011
Karma: 0
Senior Member
Done.
Re: Install.php suggestion [message #165412 is a reply to message #165407] Tue, 14 June 2011 04:23 Go to previous message
naudefj is currently offline  naudefj   South Africa
Messages: 3771
Registered: December 2004
Karma: 28
Senior Member
Administrator
Core Developer
Great. Thanks!
  Switch to threaded view of this topic Create a new topic Submit Reply
Previous Topic: JQuery Update
Next Topic: Help on -- User Selectable language
Goto Forum:
  

-=] Back to Top [=-
[ Syndicate this forum (XML) ] [ RSS ]

Current Time: Thu Nov 28 06:59:39 GMT 2024

Total time taken to generate the page: 0.02204 seconds