comanage-dev - Re: [comanage-dev] Updated Coding Guidelines
Subject: COmanage Developers List
List archive
- 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:
"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-
>>
>>
>
"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-
>>
>>
>
- [comanage-dev] Updated Coding Guidelines, Benn Oshrin, 12/15/2011
- Re: [comanage-dev] Updated Coding Guidelines, Scott Koranda, 12/15/2011
- Re: [comanage-dev] Updated Coding Guidelines, Benn Oshrin, 12/16/2011
- Re: [comanage-dev] Updated Coding Guidelines, Scott Koranda, 12/16/2011
- Re: [comanage-dev] Updated Coding Guidelines, Marie Huynh, 12/16/2011
- Re: [comanage-dev] Updated Coding Guidelines, Scott Koranda, 12/16/2011
- Re: [comanage-dev] Updated Coding Guidelines, Benn Oshrin, 12/16/2011
- Re: [comanage-dev] Updated Coding Guidelines, Benn Oshrin, 12/17/2011
- Message not available
- Message not available
- Re: [comanage-dev] Updated Coding Guidelines, Marie Huynh, 12/20/2011
- Re: [comanage-dev] Updated Coding Guidelines, Benn Oshrin, 12/20/2011
- Re: [comanage-dev] Updated Coding Guidelines, Marie Huynh, 12/20/2011
- Re: [comanage-dev] Updated Coding Guidelines, Benn Oshrin, 12/20/2011
- Message not available
- Re: [comanage-dev] Updated Coding Guidelines, Scott Koranda, 12/16/2011
- Re: [comanage-dev] Updated Coding Guidelines, Marie Huynh, 12/16/2011
- Re: [comanage-dev] Updated Coding Guidelines, Scott Koranda, 12/16/2011
- Message not available
- Re: [comanage-dev] Updated Coding Guidelines, Benn Oshrin, 12/20/2011
- Re: [comanage-dev] Updated Coding Guidelines, Benn Oshrin, 12/16/2011
- Re: [comanage-dev] Updated Coding Guidelines, Scott Koranda, 12/15/2011
Archive powered by MHonArc 2.6.16.