Skip to content

18.0 training mdoy #817

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

Draft
wants to merge 15 commits into
base: 18.0
Choose a base branch
from
Draft

18.0 training mdoy #817

wants to merge 15 commits into from

Conversation

martiyen
Copy link

No description provided.

@robodoo
Copy link

robodoo commented Jun 18, 2025

Pull request status dashboard

@@ -0,0 +1 @@
from . import models

Choose a reason for hiding this comment

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

We try to include the return character on the last line in every file.
This way when adding more lines later, the previously last line is not impacted in the diff.

Comment on lines 19 to 21
garden_orientation = fields.Selection(
string='Type',
selection=[('north', 'North') ,('south', 'South'), ('east', 'East'), ('west', 'West')],

Choose a reason for hiding this comment

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

In python, we try to follow the following convention for quoting strings:

  • single quote for technical terms
  • double quotes for displayed texts

=> ('north', "North")

Let's try to apply it during this exercise, even though the existing codebase contains many counter examples.

Copy link

@bso-odoo bso-odoo left a comment

Choose a reason for hiding this comment

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

Here are some comments:

@@ -2,8 +2,8 @@

Choose a reason for hiding this comment

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

Commit messages are supposed to be truncated at 72 columns, so, this one is bit too long.
But after a blank line you can put as much text as you want.

Anyway, do not worry about that, instead let's take this opportunity for some git training.
Try to make these fixes directly within the commits where the lines were introduces.
Look at the git rebase -i command to rewrite the history on your branch, and then you'll need to git push --force-with-lease to send such a change to the repo. (Try not to use git push --force to avoid accidents when you will be working with other people on a common branch)

@@ -1 +1 @@
from . import estate_property
from . import estate_property, estate_property_type, estate_property_tag, estate_property_offer

Choose a reason for hiding this comment

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

We try to have imports in __init__.py files separated on distinct lines.
Always the same reason, to reduce the diff when making future changes.

@martiyen martiyen force-pushed the 18.0-training-mdoy branch from fd226d5 to 91b868c Compare June 18, 2025 14:23
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