documentation icon indicating copy to clipboard operation
documentation copied to clipboard

Master template guideline seb

Open seb-odoo opened this issue 3 years ago • 7 comments

seb-odoo avatar Jun 30 '21 16:06 seb-odoo

Hello, no offense taken, I'm surprised to see so many negative comments now, because according to AL everybody was verbally in agreement for the most part, but I guess that was a miscommunication.

At least I'm glad that you looked into it, hopefully we can work towards something that works for all of us.

I'll address comments separately.

seb-odoo avatar Jul 06 '21 11:07 seb-odoo

I believe a good guidelines should not be too strict; often, common sense and the context are more important than strong guidelines. And we can trust developers to do well.

For example, qweb on the tag makes it obvious that you loop on the <tr>:

<tr t-foreach="..." t-as="...">
    <td></td>
</tr>

or here, when the "if" is related to an alert dialog: it makes the code more readable

<div t-if="..." class="alert alert-primary">
     Please check this alert.
</div>

But in other context, I agree that a separate <t> is more clean:

<t t-if="...">
    <div>a mix of different things</div>
    <div>bla</div>
</t>

Another exemple, even though I agree to have attributes on several lines, I might understand someone that put t-forach and t-as on the same line (because in this context it makes sense):

<div
   clas="myclass"
   t-forach="..." t-as="..."
>

It's difficult to have strict rules that are good in every context.

I believe a good guideline is a guideline on which most everyone agrees. For subjective guidelines where everyone has a different point of view, it's better to let people free to do what makes the most sense according to the context, and avoid having a guideline that will frustrate some.

fpodoo avatar Jul 06 '21 16:07 fpodoo

a good guideline is difficult @seb-odoo It is clearly difficult to find common ground. Sorry to make your life harder, but this is the kind of situation where we can easily lose so much time refusing/reviewing PR, frustrating devs, asking for insignificant template changes, or writing/installing/running custom tooling.

In this case, here are the only parts of the spec that I think would bring value to the odoo ecosystem:

  • a standard way to name custom css classes: prefix o-, and a sequence of lowercase separated by - such as o-form-view
  • a recommendation to put the css specific to a component in a file next to it
  • standard indentation fixed to 4 spaces, make sure closing tag is indented exactly like opening tag
  • use bootstrap standard utility class as much as possible (unless it is getting ridiculous, but then, common sense should be applied)

Also, this one may be acceptable:

  • all sub parts of a component template that need to be targetted by css could have a standard way to be named, with the main component as prefix. So, as an hypothetical example, the search view in the control panel could have o-control-panel-search-bar instead of o-search-bar

ged-odoo avatar Jul 06 '21 18:07 ged-odoo

I have updated the PR according to feedback:

  • removed rules that clearly didn't get consensus
  • changed some rules to be suggestions instead of requirements
  • adapted some phrases/wording
  • added examples
  • followed 100 characters limit per line guideline for rst writing

Hopefully this should be better for everyone, let me know.

seb-odoo avatar Jul 07 '21 15:07 seb-odoo

@seb-odoo Please request a review from odoo/doc-review when everybody is ok with the content.

AntoineVDV avatar Jul 08 '21 09:07 AntoineVDV

That's not what I meant @C3POdoo.

AntoineVDV avatar Dec 09 '21 16:12 AntoineVDV

That's not what I meant @ C3POdoo.

Being subscribed once is not enough, you have to ping this poor bot so I receive every email in double, why so much hate ?

mart-e avatar Dec 09 '21 16:12 mart-e