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

Home » FUDforum Development » Plugins and Code Hacks » Install.php suggestion
Show: Today's Messages :: Unread Messages :: Show Polls :: Message Navigator
| Subscribe to topic | Bookmark topic 
Switch to threaded view of this topic Create a new topic Submit Reply
Install.php suggestion [message #165402] Sat, 11 June 2011 13:43 Go to next message
Dayo is currently offline  Dayo   
Messages: 101
Registered: April 2011
Karma: 0
Senior Member
add to buddy list
ignore all messages by this user
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: Sat, 11 June 2011 20:37]

Report message to a moderator

Re: Install.php suggestion [message #165403 is a reply to message #165402] Sun, 12 June 2011 00:40 Go to previous messageGo to next message
naudefj is currently offline  naudefj   South Africa
Messages: 3631
Registered: December 2004
Karma: 17
Senior Member
Administrator
Core Developer
add to buddy list
ignore all messages by this user
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 03:13 Go to previous messageGo to next message
Dayo is currently offline  Dayo   Bahrain
Messages: 101
Registered: April 2011
Karma: 0
Senior Member
add to buddy list
ignore all messages by this user
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 00:36 Go to previous messageGo to next message
naudefj is currently offline  naudefj   South Africa
Messages: 3631
Registered: December 2004
Karma: 17
Senior Member
Administrator
Core Developer
add to buddy list
ignore all messages by this user
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 09:27 Go to previous messageGo to next message
Dayo is currently offline  Dayo   
Messages: 101
Registered: April 2011
Karma: 0
Senior Member
add to buddy list
ignore all messages by this user
Done.
Re: Install.php suggestion [message #165412 is a reply to message #165407] Tue, 14 June 2011 00:23 Go to previous message
naudefj is currently offline  naudefj   South Africa
Messages: 3631
Registered: December 2004
Karma: 17
Senior Member
Administrator
Core Developer
add to buddy list
ignore all messages by this user
Great. Thanks!
Quick Reply
Formatting Tools:   
  Switch to threaded view of this topic Create a new topic
Previous Topic: JQuery Update
Next Topic: Help on -- User Selectable language
Goto Forum:
  

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

Current Time: Tue Dec 12 01:48:30 EST 2017

Total time taken to generate the page: 0.00733 seconds