acsone / bobtemplates.odoo

A set of mr.bob templates to use when developing Odoo addons.
GNU Lesser General Public License v3.0
48 stars 47 forks source link

[IMP] - remove view name #70

Closed sbejaoui closed 1 year ago

sbejaoui commented 1 year ago

Since version 12, Odoo automatically generates the name of a view if it is not specified in the XML definition. Additionally, the external ID is displayed, so we can determine in which module this view is defined.

image

adrienpeiffer commented 1 year ago

I find it a little confusing to not name the view, but that's certainly a matter of taste. Any other opinions?

sbidoul commented 1 year ago

I personally always struggled to give meaningful names to views. So since Odoo displays all we need in most cases (i.e. when there is only one view per model per module), and the name we generate does not really add value, I'm fine to leave it out.

IT-Ideas commented 1 year ago

I find it a little confusing to not name the view, but that's certainly a matter of taste. Any other opinions?

Having been part of the R&D at odoo from version 14.0 up to 16.0, I have the same feeling, we've always named views.

sbidoul commented 1 year ago

@adrienpeiffer why do you find it confusing to not name views ? @IT-Ideas what is the rationale at Odoo to name views manually and is there a naming convention ?

IT-Ideas commented 1 year ago

@adrienpeiffer why do you find it confusing to not name views ? @IT-Ideas what is the rationale at Odoo to name views manually and is there a naming convention ?

I'd say that the rationale is that it could/should have been used in order to help the developer guess what the view is about (especially in inheritance context). BUT I just jumped into the code and, although we always did it, it seems indeed most of the time almost a duplicate of the external id or equal to what is displayed in the caption. So I'd agree with you guys 🙂

adrienpeiffer commented 1 year ago

A matter of taste. In most cases it's not necessary. It can be added manually if required.

adrienpeiffer commented 1 year ago

For the time being, Odoo version 8 and above is supported. If I understand correctly, with the change, generated views will no longer work for versions prior to 12 ?

sbejaoui commented 1 year ago

For the time being, Odoo version 8 and above is supported. If I understand correctly, with the change, generated views will no longer work for versions prior to 12 ?

well spotted! I made the change,

codecov-commenter commented 1 year ago

Codecov Report

Merging #70 (dfeed02) into master (8843291) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #70   +/-   ##
=======================================
  Coverage   90.38%   90.38%           
=======================================
  Files           2        2           
  Lines         156      156           
  Branches       24       24           
=======================================
  Hits          141      141           
  Misses          6        6           
  Partials        9        9           
adrienpeiffer commented 1 year ago

@sbejaoui Thanks. There is an issue with isort on precommit. Could you rebase on master ?

sbejaoui commented 1 year ago

@sbejaoui Thanks. There is an issue with isort on precommit. Could you rebase on master ?

done

adrienpeiffer commented 1 year ago

Thanks @sbejaoui