Skip to content

Onboarding Server framework 101#1280

Open
4yuub wants to merge 8 commits into
odoo:19.0from
odoo-dev:19.0-onboarding-aykar
Open

Onboarding Server framework 101#1280
4yuub wants to merge 8 commits into
odoo:19.0from
odoo-dev:19.0-onboarding-aykar

Conversation

@4yuub
Copy link
Copy Markdown
Member

@4yuub 4yuub commented May 19, 2026

No description provided.

@robodoo
Copy link
Copy Markdown

robodoo commented May 19, 2026

Pull request status dashboard

@4yuub 4yuub requested a review from thbo-odoo May 19, 2026 14:55
@4yuub 4yuub changed the title chore: onboarding Server framework 101 chapter 1 to 5 Onboarding Server framework 101 May 19, 2026
@4yuub 4yuub requested review from Megaaaaaa and removed request for thbo-odoo May 19, 2026 15:17
@4yuub 4yuub force-pushed the 19.0-onboarding-aykar branch from c0d88d0 to f310ea3 Compare May 19, 2026 15:23
@4yuub 4yuub force-pushed the 19.0-onboarding-aykar branch from f310ea3 to 4bb98e2 Compare May 20, 2026 07:18
Copy link
Copy Markdown

@Megaaaaaa Megaaaaaa left a comment

Choose a reason for hiding this comment

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

Hello 👋

Here's a first review for you, that's already a really nice start 👍

There's quite a lot of stuff but don't worry about it, it's a lot of the same small things that repeat themselves.

When receiving a review, you can go through all comments and apply what is suggested or bring your own alternative. Keep in mind that a review is not the utlimate solution, it's only someone seeing things he would have done differently and offering an alternative. You're always free to answer with an other alternative or even say you disagree and bring your arguments.

What I would suggest you do and continue to do while you're not the most comfortable with odoo's structure is to not apply all the changes at once. For example, changing a model's _name has a lot of impact and if you change everything at once, you might end up with a lot of errors when running the database. Splitting the review in multiple steps and trying to run your db in between those can save you a lot of time in the long run.

Also a quick tip for the methodology when applying reviews changes: What I like to do it put a reaction like a thumbsup on a message once I have made the change locally. Then when everything is marked with something, I push, go through my diff and mark as resolved the things that are now outdated so nothing is left forgotten.

If you have any question, don't hesitate to ask!

Comment thread estate/models/__init__.py Outdated
from . import estate_property
from . import estate_property_type
from . import estate_property_tag
from . import estate_property_offer No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Every file must end with a line break so that if someone adds something after your last line, he doesn't alter the history of your line by adding it 👍

Suggested change
from . import estate_property_offer
from . import estate_property_offer

This is also known as the "EOL", the End Of Line character, I'll only tell you "EOL" for the other occurences 😄

Comment thread estate/models/__init__.py
@@ -0,0 +1,5 @@
# flake8: noqa: F401
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You don't need this 👍

Comment thread estate/models/__init__.py Outdated
Comment on lines +2 to +5
from . import estate_property
from . import estate_property_type
from . import estate_property_tag
from . import estate_property_offer No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Depending on the team you join, they might ask you to order your imports alphabetically so you might aswell start now, just in case (it's highly probable tbh) 😄

Suggested change
from . import estate_property
from . import estate_property_type
from . import estate_property_tag
from . import estate_property_offer
from . import estate_property
from . import estate_property_offer
from . import estate_property_tag
from . import estate_property_type

Comment on lines +1 to +4
from odoo import models, fields, api, exceptions
from odoo.tools import float_compare, float_is_zero
from datetime import date
from dateutil.relativedelta import relativedelta
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Every single team will ask to do this one: order the imports. Modules alphabetically and attributes asciibetically. It's clearer in the coding guidelines with an example 😄

Suggested change
from odoo import models, fields, api, exceptions
from odoo.tools import float_compare, float_is_zero
from datetime import date
from dateutil.relativedelta import relativedelta
from datetime import date
from dateutil.relativedelta import relativedelta
from odoo import api, exceptions, fields, models
from odoo.tools import float_compare, float_is_zero

from dateutil.relativedelta import relativedelta


def three_months_from_now(_model):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think we would more often see:

Suggested change
def three_months_from_now(_model):
def three_months_from_now(self):

even if you don't use self in the method. But what we would see evne more is this being done in the field declaration directly

    date_availability = fields.Date(copy=False, default=lambda x: date.today() + relativedelta(months=3))

It's up to you 👍

Comment thread estate/__manifest__.py
'name': "Real Estate",
'version': '1.0',
'depends': ['base'],
'author': "Odoo S.A. (aykar)",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

As an odoo employee you cannot sign modules in your own name, only in Odoo's

Suggested change
'author': "Odoo S.A. (aykar)",
'author': "Odoo S.A.",

Comment thread estate/__manifest__.py
'installable': True,
'application': True,
'liscence': 'LGPL-3',
# data files always loaded at installation
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
# data files always loaded at installation

Comment thread estate/__manifest__.py
'views/estate_property_offers_views.xml',
'views/estate_menus.xml',
],
# data files containing optionally loaded demonstration data
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
# data files containing optionally loaded demonstration data

Comment thread estate/__manifest__.py
],
# data files containing optionally loaded demonstration data
'demo': [
# 'demo/demo_data.xml',
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I guess this will be useful later but becareful to not push fragments of features because you might end up forgetting them 👍

Comment thread estate/__manifest__.py Outdated
""",
'installable': True,
'application': True,
'liscence': 'LGPL-3',
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

small typo

Suggested change
'liscence': 'LGPL-3',
'licence': 'LGPL-3',

Megaaaaaa

This comment was marked as duplicate.

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.

3 participants