Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IMP] tutorials: first commit #353

Open
wants to merge 19 commits into
base: 18.0
Choose a base branch
from

Conversation

roto-odoo
Copy link

modules: tutorials/awesome_clicker
files: init.py

first commit following the instructions of the tutorials

modules: tutorials/awesome_clicker
files: __init__.py

first commit following the instructions of the tutorials
@robodoo
Copy link

robodoo commented Feb 17, 2025

Pull request status dashboard

Copy link

@clbr-odoo clbr-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job so far, keep up 👍

@@ -0,0 +1,4 @@
# -*- coding: utf-8 -*-

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not necessary in Python files for a long time already.
There are some that are still there in the code for historical reason, but please don't add them on new files 🙂
AFAIK, the licensing is also not really necessary to add on each file since it's defined in the manifest for the whole module.

# Part of Odoo. See LICENSE file for full copyright and licensing details.
from dateutil.relativedelta import relativedelta
from odoo import fields, models

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two empty lines between imports and class.

name = fields.Char(required=True)
description = fields.Text()
postcode = fields.Char()
date_availability = fields.Date(default=fields.Date.today()+relativedelta(months=3), copy=False)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small note:
fields.Date.today() will actually give you the server time, if you want to have the actual today of the user (which is the case most of the time for business logic) you'll have to use context_today.

Comment on lines 26 to 31
state = fields.Selection(
selection=[('new', 'New'), ('offer_received', 'Offer Received'), ('offer_accepted', 'Offer Accepted'),
('sold', 'Sold'), ('cancelled', 'Cancelled')],
copy=False,
default='new',
required=True

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
state = fields.Selection(
selection=[('new', 'New'), ('offer_received', 'Offer Received'), ('offer_accepted', 'Offer Accepted'),
('sold', 'Sold'), ('cancelled', 'Cancelled')],
copy=False,
default='new',
required=True
state = fields.Selection(
selection=[
('new', 'New'),
('offer_received', 'Offer Received'),
('offer_accepted', 'Offer Accepted'),
('sold', 'Sold'), ('cancelled', 'Cancelled'),
],
copy=False,
default='new',
required=True,

Don't hesitate to use the space if it makes it more readable. Also it's a good practice to add a , even on the last parameter in such case: if someone needs to add something later on, he won't have to modify the last line to add it, and that will keep the git history very clean.

@@ -0,0 +1,7 @@
<odoo>
<menuitem id="estate_menu_root" name="Real Estate">
<menuitem id="estate_first_level_menu" name="Advertisements">

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also have convention for xml ids, although they are quite recent and therefore not that often enforced in the code for the moment.
I suggest you read this section again and update them accordingly 🙂
https://www.odoo.com/documentation/master/contributing/development/coding_guidelines.html#xml-ids-and-naming

Comment on lines 1 to 5
# -*- coding: utf-8 -*-

#TEST COMMIT

print("test")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove this now 😄

Comment on lines 39 to 41
partner_id = fields.Many2one(
'res.partner', string='Buyer',
)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inline all of those. Let's stay consistent with the indentation we did previously.

<field name="bedrooms"/>
<field name="living_area" string="Living Area (sqm)"/>
<field name="facades"/>
<filter name="available" string="Available" domain="['|', ('state', '=', 'new'), ('state', '=', 'offer_received')]"/>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<filter name="available" string="Available" domain="['|', ('state', '=', 'new'), ('state', '=', 'offer_received')]"/>
<filter name="available" string="Available" domain="['('state', 'in', ('new', 'offer_received'))]"/>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants