Re: Capitalization
Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> Mon, 18 Mar 2013 16:33:02 +0100
| Newsgroups | gmane.comp.web.gallery.devel |
|---|---|
| Message-ID | <CA+z51A4h9_nLTkbkjNWfzi8W8FSmKkf1yddFxWOjQ=Ui3rpKyQ@mail.gmail.com> |
Hey everyone,
There's a few class names that Gallery already overloads in special
ways, and I was wondering how we want to translate them to K3.
tl;dr: unless someone says otherwise, I'm likely going to call them
[Module]_Module_[Type]_[Module]. Example: tag_event_Core, which is
transparently extended as tag_event, would be moved to
Tag_Module_Event_Tag, which would be transparently extended as
Module_Event_Tag.
We have some helper class patterns:
[module]_installer
[module]_event
[module]_theme
[module]_block
[module]_task
[module]_rss
[module]_rest
And one controller class pattern:
Admin_[Something]_Controller
Translating the controller class pattern is fairly obvious: it should
be [Module]_Controller_Admin_[Something], which is then transparently
extended as Controller_Admin_[Something]. This flows nicely with K3.
Translating the helper class patterns is slightly less obvious. Here
are some possibilities:
1. Call them [Module]_Module_[Type], e.g. tag_event_Core =>
Tag_Module_Event. The asterisk to this approach is that, unlike every
other class in the classes directory, these should *not* get the
transparent-extension-enabling class added to it (e.g. "class
Module_Event extends Tag_Module_Event {}").
2. Call them [Module]_[Module]_[Type], e.g. tag_event_Core =>
Tag_Tag_Event. This is similar to the previous approach, but keeps
the transparent-extension-enabling class added to it (e.g. "class
Tag_Event extends Tag_Tag_Event {}"). This means that, in the end,
it's called by other functions as Tag_Event. The asterisk to this
approach is that the directory structure is a little broken. In this
example, the tag module would have all of its "real" class code in
modules/tag/classes/Tag, which are transparently extended by files in
the parent of that directory. So, the event code would be in
/modules/tag/classes/Tag/Tag/Event.php, and the
transparent-extension-enabling file at
/modules/tag/classes/Tag/Event.php, with all of its siblings being
"real" classes instead of the thin extensions. It works, but it
doesn't seem clean.
3. Call them Module_[Module]_[Type], e.g. tag_event_Core =>
Module_Tag_Event. This has the same problem as 2, in an even
less-idiomatic way.
4. Call them [Module]_[Type]_[Module], e.g. tag_event_Core =>
Tag_Event_Tag, which is then transparently extended as Event_Tag.
This doesn't seem like a great idea since examples like Rest_Gallery
from the gallery module seem likely to clobber something in the rest
module.
5. Call them [Module]_Module_[Type]_[Module], e.g. tag_event_Core =>
Tag_Module_Event_Tag, which is then transparently extended as
Module_Event_Tag. This flows pretty well with K3, and is my current
favorite approach.
6. Put them somewhere else and bypass the standard autoloader. This
has the advantage of allowing us to make the class names look less
encumbered (e.g. tag_event_Core => tag_event), but otherwise seems to
intentionally break all of the other Kohana conventions.
Thoughts?
Take care,
Shad
On 17 March 2013 23:23, Bharat Mediratta <[email protected]> wrote:
> cc: devel
>
> We're going to have to go through the entire app file by file and fix
> everything. Unit tests will help. So I think the best way to start is to
> move everything to its rightful new place and then start fixing things.
> Some things we'll need to sort out - like all of the MY_ overloads we have
> in the gallery module to deal with issues in K2 will have to be evaluated on
> a case-by-case basis. When in doubt, mercilessly remove stuff and note it
> in the shared doc and we'll circle back and fix it later.
>
> great work!
>
> -Bharat
>
>
> On Sun, Mar 17, 2013 at 2:42 PM, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote:
>>
>> OK, all set.
>>
>> At this point, all files in modues/gallery/classes follow the new
>> conventions. I've systematically changed:
>> - directory/file names
>> - class declarations
>> - transparent class extension files
>>
>> So, we have 52 files, each of which has a "real" file and it's
>> transparent-extension-enabling helper (104 in total). I haven't yet
>> changed any references to these new files, so I'm fairly certain I've
>> broken a fair number of things... but we have to rip off the band-aid
>> at some point, and I figured getting the files in the right place
>> sooner than later was useful.
>>
>> In the spirit of being systematic, I also did each of these in three
>> separate commits. My intention was to be ultra-mega-super careful
>> with file histories, and from what I understand, git log --follow is
>> most reliable when a rename is accompanied by zero other changes.
>>
>> I haven't yet tackled the rest of the helpers and libraries, but will
>> start to take a peek tomorrow. After that, I can either:
>> - jump into doing the same task on the other modules.
>> - try and systematically fix all the broken references to K2-named
>> classes in the gallery module.
>>
>> Thoughts on which makes sense to be done first? Also, any other thoughts?
>>
>> Take care,
>> Shad
>>
>>
>> On 17 March 2013 22:03, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote:
>> > Quick heads-up: I'm working on systematically renaming, moving,
>> > transparently extending, etc. all of the gallery module controllers
>> > and models... in other words, if possible, hold off on commits for
>> > controllers and models for a sec otherwise we're pretty likely to
>> > collide.
>> >
>> > On 17 March 2013 17:06, Bharat Mediratta <[email protected]> wrote:
>> >> (cc devel)
>> >>
>> >> If we don't do this now we may very well have to do it later which will
>> >> result in another large non-backwards-compatible event so let's do it
>> >> now.
>> >> For the most part its an easy change to make for contributors...
>> >>
>> >> -Bharat
>> >>
>> >> On Mar 17, 2013 2:32 AM, "Shad Laws" <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote:
>> >>>
>> >>> Hey Bharat,
>> >>>
>> >>> So, I sat down this morning with a cup of coffee, ready to move
>> >>> forward on this convention and start moving things around and making
>> >>> the transparent extension files...
>> >>>
>> >>> ... and then read a bit more and started scratching my head again.
>> >>>
>> >>> Apparently, the reason why it isn't totally clear in the Kohana docs
>> >>> is because it's new to 3.3, and half of the Kohana community is still
>> >>> confused and/or conflicted about it. There's one camp, for whom the
>> >>> idea of lowercasing all filenames makes sense (and still works with
>> >>> Kohana::auto_load_lowercase()), and there's the other, for whom the
>> >>> idea of standardizing with PSR-0 like Symfony and Zend seems like the
>> >>> way to go. Note that because the "big boys" have decided upon PSR-0
>> >>> and its case-sensitive and capitalized standard, they're considering
>> >>> putting a PSR-0-compliant autoloader in the PHP core in the future.
>> >>>
>> >>> So, it seems we have three options:
>> >>> 1. Keep our lowercase filenames *and* class names. This helps
>> >>> backward compatibility, but as you pointed out might be awkward given
>> >>> how the system directory is built.
>> >>> 2. Keep our lowercase filenames, but capitalize many of the class
>> >>> names and use auto_load_lowercase(). This helps backward
>> >>> compatibility, but doesn't seem to be a very clean approach.
>> >>> 3. Make everything case-sensitive and capitalized. This is in sync
>> >>> with the Kohana system directory, the decisions of Zend and Symfony,
>> >>> the PSR-0 autoloader, and (maybe) the future of PHP. It's also the
>> >>> decision that's the least backward compatible.
>> >>>
>> >>> After spending more time researching and thinking about this, I'm
>> >>> inclined to reverse my position... if we're upgrading to K3, we may as
>> >>> well finish the rip-off-the-bandaid-quickly task and do what seems the
>> >>> most future-proof. Thoughts?
>> >>>
>> >>> Take care,
>> >>> Shad
>> >>>
>> >>>
>> >>> On 17 March 2013 02:20, Bharat Mediratta <[email protected]> wrote:
>> >>> >
>> >>> > Sounds good to me.
>> >>> >
>> >>> >
>> >>> > On Sat, Mar 16, 2013 at 9:39 AM, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]>
>> >>> > wrote:
>> >>> >>
>> >>> >> ...and it looks like, at least in the controller docs, they go so
>> >>> >> far
>> >>> >> as
>> >>> >> to require lowercase filenames:
>> >>> >>
>> >>> >> http://kohanaframework.org/3.0/guide/kohana/mvc/controllers
>> >>> >>
>> >>> >> Sent from my Swiss Army Phone
>> >>> >>
>> >>> >> On Mar 16, 2013 3:54 PM, "Shad Laws" <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote:
>> >>> >>>
>> >>> >>> Yep, it's Kohana:
>> >>> >>> http://kohanaframework.org/3.3/guide/kohana/tips
>> >>> >>>
>> >>> >>> Sent from my Swiss Army Phone
>> >>> >>>
>> >>> >>> On Mar 16, 2013 3:30 PM, "Shad Laws" <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote:
>> >>> >>>>
>> >>> >>>> Hey Bharat,
>> >>> >>>>
>> >>> >>>> I haven't investigated further just yet, but I suspect your guess
>> >>> >>>> is
>> >>> >>>> right: it's the Kohana autoloader that cares, not PHP.
>> >>> >>>>
>> >>> >>>> My current thought is that it seems it's largely a case of
>> >>> >>>> six-or-half-a-dozen. In that case, I'm inclined to give the tie
>> >>> >>>> breaker to
>> >>> >>>> backward compatibility: if modules can still call
>> >>> >>>> ORM::factory("item"), then
>> >>> >>>> that's one less thing we break with contrib projects. I think
>> >>> >>>> renaming the
>> >>> >>>> classes to be consistently capitalized is good (and will need to
>> >>> >>>> happen with
>> >>> >>>> contrib stuff, too), but I'm personally happy to keep the
>> >>> >>>> filenames
>> >>> >>>> lowercase.
>> >>> >>>>
>> >>> >>>> Thoughts?
>> >>> >>>>
>> >>> >>>> Take care,
>> >>> >>>> Shad
>> >>> >>>>
>> >>> >>>> Sent from my Swiss Army Phone
>> >>> >>>>
>> >>> >>>> On Mar 16, 2013 4:51 AM, "Bharat Mediratta" <[email protected]>
>> >>> >>>> wrote:
>> >>> >>>>>
>> >>> >>>>>
>> >>> >>>>> So I experimented with this a bit - and it turns out that models
>> >>> >>>>> are
>> >>> >>>>> case sensitive in the ORM code.
>> >>> >>>>>
>> >>> >>>>> modules/gallery/classes/Model/item.php contains:
>> >>> >>>>> class Model_Item extends ORM { }
>> >>> >>>>>
>> >>> >>>>> and is accessible with ORM::factory("item") but not
>> >>> >>>>> ORM::factory("Item"). Doesn't matter if you call the class
>> >>> >>>>> Model_Item or
>> >>> >>>>> Model_item.
>> >>> >>>>>
>> >>> >>>>> So if we're going to be consistent with capitalization, we may
>> >>> >>>>> need
>> >>> >>>>> to
>> >>> >>>>> rename that file to Item.php and then refer to the model as
>> >>> >>>>> "Item"
>> >>> >>>>> in the
>> >>> >>>>> code. Or we can leave it lower case and refer to it as "item".
>> >>> >>>>> Not
>> >>> >>>>> sure
>> >>> >>>>> yet, and we don't have to decide now - just pointing it out.
>> >>> >>>>>
>> >>> >>>>> My guess is that this is an issue with the autoloader, because I
>> >>> >>>>> *think* that PHP is case insensitive when it comes to class
>> >>> >>>>> names
>> >>> >>>>> but I'm
>> >>> >>>>> not 100% sure.
>> >>> >>>>>
>> >>> >>>>>
>> >>> >>>>>
>> >>> >>>>>
>> >>> >>>>>
>> >>> >>>>>
>> >>> >>>>> On Fri, Mar 15, 2013 at 5:23 PM, Bharat Mediratta
>> >>> >>>>> <[email protected]>
>> >>> >>>>> wrote:
>> >>> >>>>>>
>> >>> >>>>>>
>> >>> >>>>>> Ah, I didn't see that and was following what I saw in the
>> >>> >>>>>> system
>> >>> >>>>>> directory.. which seems at odds with their style guide. I
>> >>> >>>>>> think we
>> >>> >>>>>> should
>> >>> >>>>>> follow their code (ie capitalize file names) as opposed to
>> >>> >>>>>> their
>> >>> >>>>>> style guide
>> >>> >>>>>> since it'll be more internally consistent with what's already
>> >>> >>>>>> there.
>> >>> >>>>>> Thoughts?
>> >>> >>>>>>
>> >>> >>>>>>
>> >>> >>>>>> On Fri, Mar 15, 2013 at 5:04 PM, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]>
>> >>> >>>>>> wrote:
>> >>> >>>>>>>
>> >>> >>>>>>> Hey Bharat,
>> >>> >>>>>>>
>> >>> >>>>>>> Started taking a look:
>> >>> >>>>>>> http://kohanaframework.org/3.0/guide/kohana/conventions
>> >>> >>>>>>>
>> >>> >>>>>>> and noticed that they recommend lowercase directory and
>> >>> >>>>>>> filenames.
>> >>> >>>>>>> I
>> >>> >>>>>>> noticed you started changing them to capitalized... thoughts?
>> >>> >>>>>>>
>> >>> >>>>>>> Shad
>> >>> >>>>>>>
>> >>> >>>>>>> Sent from my Swiss Army Phone
>> >>> >>>>>>
>> >>> >>>>>>
>> >>> >>>>>
>> >>> >
>> >>>
>> >>
>>
>
------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_mar
__[ 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 ]