Re: Gallery's K3 class name/dir/file conventions (was "Capitalization")

Bharat Mediratta <[email protected]> Tue, 26 Mar 2013 09:41:18 -0700
Newsgroups gmane.comp.web.gallery.devel
Message-ID <CAESa+_mqOcbhA_4A558SS=C1oAgNuWWrAVRh_SV=Pz_2MGU+ow@mail.gmail.com>
Outstanding.  This advances the issue I've been (slowly!) working on which
is to sort out the identity provider code.  I suspect that we want to
overhaul it, but there will be so many other issues that I'd like to get it
working for now before we redesign it.

-Bharat


On Tue, Mar 26, 2013 at 5:57 AM, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote:

> Hey everyone,
>
> I just finished performing the same class/file move/rename/search/replace
> operation on the user module.  Also, I made some tweaks to the gallery
> module to reflect this (in particular with IdentityProvider and associated
> files) and updated PHPass.
>
> At this point, I *think* all of the files in the gallery and user modules
> should talk to each other - all of the interfaces, extensions, classes, and
> abstractions seem to be consistent between the two!  All of the
> class/filename changes are documented in the Google spreadsheet.  I've also
> compiled a little summary at the end of this email.
>
> While this process is fresh in my mind and I'm on a roll, I'm probably
> going to the same thing for the rest of the modules before diving into
> Formo...
>
> Take care,
> Shad
>
> ----
>
> How Identity and IdentityProvider got translated (typical cases):
>
> Old class: /modules/gallery/helpers/identity.php --> class identity_Core
> New class: /modules/gallery/classes/Gallery/Identity.php --> class
> Gallery_Identity
> Extension: /modules/gallery/classes/Identity.php --> class Identity
> extends Gallery_Identity
>
> Old class: /modules/gallery/libraries/IdentityProvider.php --> class
> IdentityProvider_Core
> New class: /modules/gallery/classes/Gallery/IdentityProvider.php --> class
> Gallery_IdentityProvider
> Extension: /modules/gallery/classes/IdentityProvider.php --> class
> IdentityProvider extends Gallery_IdentityProvider
>
> How IdentityProvider_Driver stuff got translated (the main driver keeps
> its class name):
>
> Old class: /modules/gallery/libraries/drivers/IdentityProvider.php -->
> interface IdentityProvider_Driver
> New class: /modules/gallery/classes/Gallery/IdentityProvider/Driver.php
> --> interface Gallery_IdentityProvider_Driver
> Extension: /modules/gallery/classes/IdentityProvider/Driver.php -->
> interface IdentityProvider_Driver extends Gallery_IdentityProvider_Driver
>
> Old class: /modules/user/libraries/drivers/IdentityProvider/Gallery.php
> --> class IdentityProvider_Gallery_Driver implements IdentityProvider_Driver
> New class: /modules/user/classes/User/IdentityProvider/Gallery.php -->
> class User_IdentityProvider_Gallery implements IdentityProvider_Driver
> Extension: /modules/user/classes/IdentityProvider/Gallery.php --> class
> IdentityProvider_Gallery extends User_IdentityProvider_Gallery
>
> How Group_Definition stuff got translated (removed from main driver, given
> new class name):
>
> Old class: /modules/gallery/libraries/drivers/IdentityProvider.php -->
> interface Group_Definition
> New class:
> /modules/gallery/classes/Gallery/IdentityProvider/GroupDefinition.php -->
> interface Gallery_IdentityProvider_GroupDefinition
> Extension: /modules/gallery/classes/IdentityProvider/GroupDefinition.php
> --> interface IdentityProvider_GroupDefinition extends
> Gallery_IdentityProvider_GroupDefinition
>
> Old class: /modules/user/models/group.php --> class Group_Model_Core
> extends ORM implements Group_Definition
> New class: /modules/user/classes/User/Model/Group.php --> class
> User_Model_Group extends ORM implements IdentityProvider_GroupDefinition
> Extension: /modules/user/classes/Model/Group.php --> class
> Model_Group extends User_Model_Group
>
> How User_Definition stuff got translated (removed from main driver, given
> new class name):
>
> Old class: /modules/gallery/libraries/drivers/IdentityProvider.php -->
> interface User_Definition
> New class:
> /modules/gallery/classes/Gallery/IdentityProvider/UserDefinition.php -->
> interface Gallery_IdentityProvider_UserDefinition
> Extension: /modules/gallery/classes/IdentityProvider/UserDefinition.php
> --> interface IdentityProvider_UserDefinition extends
> Gallery_IdentityProvider_UserDefinition
>
> Old class: /modules/user/models/user.php --> class User_Model_Core extends
> ORM implements User_Definition
> New class: /modules/user/classes/User/Model/User.php --> class
> User_Model_User extends ORM implements IdentityProvider_UserDefinition
> Extension: /modules/user/classes/Model/User.php --> class
> Model_User extends User_Model_User
>
>
>
> On 25 March 2013 18:08, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote:
>
>> Okay, 1 and 2 are done.  Also, the Google spreadsheet is up-to-date with
>> all of the changes.
>>
>> My search'n'replace operation managed to make about 2800 changes!
>>  Hopefully that should cut down on wrote tasks down the line :-)
>>
>> Take care,
>> Shad
>>
>>
>> On 25 March 2013 01:17, Bharat Mediratta <[email protected]> wrote:
>>
>>>
>>> All looks good for me.  If you miss a few or get a few wrong it's no big
>>> deal, we'll be able to figure them out from context and fix them up.
>>>
>>>
>>> On Sun, Mar 24, 2013 at 3:34 PM, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote:
>>>
>>>> Hey Bharat,
>>>>
>>>> Woohoo - I think we've finally arrived on the same page!  Good
>>>> conversation, though... I feel like we both figured out a lot of nuances in
>>>> the process.  Onward!
>>>>
>>>> My roadmap for the near future:
>>>> 1. Finish moving around the remaining files in the gallery module
>>>> (tonight)
>>>> 2. Fire up a module-wide grep to try and autofix as much as I can to
>>>> avoid changing every class/file name by hand (tomorrow)
>>>> 3. Do the same thing to the user module (next couple days)
>>>> 4. Start getting up to speed on Formo (afterward)
>>>>
>>>> If it helps your workflow, I can invert the order of 2 and 3...
>>>>
>>>> For #2, I could use a little input.  For class names, I'll grep for the
>>>> following patterns (not yet carefully regex-ized, but you get the idea):
>>>> 1. class::
>>>> 2. new Class
>>>>
>>>> Also, I'll look for these, but check them one by one:
>>>> 1. call_user_func, call_user_func_array
>>>> 2. $classvar::
>>>> 3. {$foo . "bar"}::
>>>> 4. new $foo
>>>> 5. new {$foo. "bar"}
>>>> 6. class_exists, method_exists
>>>>
>>>> Additionally, I'll look for other files (media, views), which I think
>>>> all occur within double or single quotes.  I may look for View( just to be
>>>> sure I got all the views.
>>>>
>>>> Any others you can think of that'd be useful to grab?
>>>>
>>>> Take care,
>>>> Shad
>>>>
>>>>
>>>> On 24 March 2013 19:34, Bharat Mediratta <[email protected]> wrote:
>>>>
>>>>>
>>>>> On Sat, Mar 23, 2013 at 3:21 PM, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote:
>>>>>
>>>>>> If we're sticking with Kohana's functions and not making a
>>>>>> wildcard-savvy loader, then this conversation is kinda moot, but for fun...
>>>>>>
>>>>>> <pseudo-philosophical tangent>
>>>>>> I feel like there are three different cases here:
>>>>>> 1. Not require a rule. (make the wildcard-savvy loader)
>>>>>> 2. Require a rule. (be opinionated and enforce one way to do it)
>>>>>> 3. Require a rule and make you define it. (extra line in module.info)
>>>>>>
>>>>>> While there are certainly arguments why 2 is better than 3, in the
>>>>>> general case, it seems like 1 could often be better than 2, no?
>>>>>> </pseudo-philosophical tangent>
>>>>>>
>>>>>
>>>>> It's not clear to me when that would be the case - so I worry that
>>>>> it's flexibility for flexibility's sake.  I'm ok with that if it doesn't
>>>>> come at a cost, but in this case it makes the explanation harder.  For
>>>>> example, it's easier for me to say "create modules/tag/classes/TagEvent.php
>>>>> and put this in it" vs. saying "create a file in module/tag/classes that
>>>>> ends in Event.php, like TagEvent.php and put this in it".  I'd probably
>>>>> just tell people to use TagEvent in that case.  So then most people would
>>>>> follow this convention - and then what would be the need for a different
>>>>> option?
>>>>>
>>>>> I'm open to this - just want to understand the value proposition.
>>>>>
>>>>>
>>>>>>  I'm a little mixed on this.  Some thoughts:
>>>>>>> - I can't tell if I like the separate subdir or not.  Frankly, that
>>>>>>> implies that there's something to separate from, but many modules can do
>>>>>>> everything they need to do with just and event handler and a block, etc.
>>>>>>>
>>>>>>
>>>>>> Personally, I do.  To me, it has a similar significance to Controller
>>>>>> or Model, in the sense that the names of their functions have special
>>>>>> meanings outside of the class itself.  This is not the case, for example,
>>>>>> for the Tag.php helper, which is why it shouldn't be in any subdir.
>>>>>>
>>>>>
>>>>> I'm definitely coming around to this idea.  I haven't heard a name
>>>>> that I like, yet though.  "Gallery" is used.  "Module" is too close to
>>>>> "Model".  "Hook" is the best I've heard but I'm mildly worried that
>>>>> there'll be cognitive conflict with the fact that some things inside that
>>>>> directory won't be "hooks" per se (tasks, etc).  But the more I think about
>>>>> it the more I like it.
>>>>>
>>>>>
>>>>>> I think by now I'm convinced of all points you just made, with the
>>>>>> sole exception being the separate directory for Gallery-specific stuff.  I
>>>>>> think it gives a meaningful distinction between it and the other stuff.
>>>>>>  Also, it'd give us the added benefit of being able to change our mind at a
>>>>>> later date should someone decide to build an even-more-optimized,
>>>>>> wildcard-capable loader.
>>>>>>
>>>>>
>>>>> Agreed.  Let's go with Hook for now and then if we come up with a
>>>>> better name before launch it should be easy to just fix up everything.
>>>>>  Onwards!
>>>>>
>>>>>
>>>>>> On a semi-related note, how do we feel about underscores in "views"
>>>>>> names?  I like the idea of adding a teeny bit of hierarchy like you did
>>>>>> with the Tag example (certainly no more than 1 level), and that removes a
>>>>>> decent number of them, but not all.  Example: is
>>>>>> /modules/gallery/views/admin/advanced_settings.php fine?  My vote is yes,
>>>>>> personally...
>>>>>>
>>>>>
>>>>> Underscores don't seem to mean anything in view names, eg:
>>>>>
>>>>>   modules/userguide/views/userguide/examples/hello_world_error.php
>>>>>
>>>>> That'd be loaded as "userguide/examples/hello_world_error.php" as far
>>>>> as I can tell.  I can't find good examples of this - but I also don't think
>>>>> that it makes sense for it to be "admin/advanced/settings.php" so I also
>>>>> vote for "admin/advanced_settings.php".  If we find a better model in the
>>>>> future, renaming all this stuff should be easy and backwards compatible.
>>>>>
>>>>> OK!  I think this means it's time to write some code?  I'm going to
>>>>> get going on seeing if I can sort out the IdentityProvider code...
>>>>>
>>>>>
>>>>
>>>
>>
>

------------------------------------------------------------------------------
Own the Future-Intel&reg; Level Up Game Demo Contest 2013
Rise to greatness in Intel's independent game demo contest.
Compete for recognition, cash, and the chance to get your game 
on Steam. $5K grand prize plus 10 genre and skill prizes. 
Submit your demo by 6/6/13. http://p.sf.net/sfu/intel_levelupd2d

__[ g a l l e r y - d e v e l ]_________________________

[ list info/archive --> http://gallery.sf.net/lists.php ]
[ gallery info/FAQ/download --> http://gallery.sf.net ]