Skip to Content.
Sympa Menu

comanage-dev - Re: [comanage-dev] Updated Coding Guidelines

Subject: COmanage Developers List

List archive

Re: [comanage-dev] Updated Coding Guidelines


Chronological Thread 
  • From: Marie Huynh <>
  • To: Benn Oshrin <>, comanage-dev <>
  • Subject: Re: [comanage-dev] Updated Coding Guidelines
  • Date: Tue, 20 Dec 2011 16:29:57 -0800

I find the first one easier to read than the second, which is what I think the current coding style says it should look like:

    var $belongsTo = array("Cou",             // A CO Org Person To is attached to one COU
                           "CoPerson" => array('className'  => 'CoPerson',
                                               'foreignKey' => 'co_person_id'
                                              ),       // A CO Org Person To is attached to one CO Person


   var $belongsTo = array(
     "Cou",             // A CO Org Person To is attached to one COU
     "CoPerson" => array(
       'className'  => 'CoPerson',
       'foreignKey' => 'co_person_id'
     ),       // A CO Org Person To is attached to one CO Person



Thoughts?

On Tue, Dec 20, 2011 at 2:54 PM, Marie Huynh <> wrote:
>
> See diff.  I didn't think the arrays with one item were hard to read so I left those.
>
>
> On Mon, Dec 19, 2011 at 10:31 PM, Marie Huynh <> wrote:
>>
>> +        $this->redirect(array('action' => 'add'));
>>
>> Does this array need to be defined before being passed?
>>
>> -                        echo '<a class="deletebutton" title="' . _txt('op.delete') . '" . _jtxt(Sanitize::html($id['identifier'])) . '\', \'' . $html->url(array('controller' => 'identifiers', 'action' => 'delete', $id['id'], 'co' => $cur_co['Co']['id'])) . '\')";>' . _txt('op.delete') . '</a>' . "\n";
>> -                        echo $html->link(_txt('op.edit'),
>> +                        print '<a class="deletebutton" title="' . _txt('op.delete') . '" . _jtxt(Sanitize::html($id['identifier'])) . '\', \'' . $html->url(array('controller' => 'identifiers', 'action' => 'delete', $id['id'], 'co' => $cur_co['Co']['id'])) . '\')";>' . _txt('op.delete') . '</a>' . "\n";
>> +                        print $html->link(_txt('op.edit'),
>>
>> I used :%s/echo/print/gc.  Do I need to fix the rest of the file too even if this ticket doesn't touch those parts?
>>
>> On Sat, Dec 17, 2011 at 5:51 PM, Benn Oshrin <> wrote:
>>>
>>> On 12/16/11 3:17 PM, Marie Huynh wrote:
>>>>
>>>> If we're going back to fix old code anyway, why not do it all at the
>>>> same time and run it through one of these formatters before tagging?
>>>>
>>>> http://coding.smashingmagazine.com/2007/07/12/time-savers-code-beautifier-and-formatter/
>>>>
>>>> Sure, they won't fix everything, but it's better than doing it all by hand.
>>>
>>>
>>> Here are my thoughts on this...
>>>
>>> I don't have a moral objection to using one of these, and if anyone wants to, by all means go ahead.
>>>
>>> I think all code commits (not just tagged releases) should be properly formatted. It's too easy to introduce bugs after testing otherwise, especially if we're relying on automated formatters. It's best to test stuff while the test cases are still fresh in your mind.
>>>
>>> Furthermore, I suspect if we defer reformatting, it'll never happen, and the codebase will become even more disjoint than it already is.
>>>
>>> On top of that, if there are problems with the new guidelines, we should discover them sooner rather than later.
>>>
>>> I'm sensitive to concerns about falling behind schedule, but there are only about 20 rules encoded in the new guidelines. It should take maybe an hour to do the reformatting, and going forward the incremental cost should be trivial.
>>>
>>> -Benn-
>>
>>
>



Archive powered by MHonArc 2.6.16.

Top of Page