-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Simple Commit for Creating a PR #192
base: 18.0
Are you sure you want to change the base?
Conversation
…ials into 18.0-tutorial-cgun
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work👌🏾. Just a few remarks on formatting/conventions.
There is a general inconsistency in the use of double and single quotes. We tend to use single quotes for technical strings and double quotes for user-facing strings.
.vscode/settings.json
Outdated
{ | ||
"workbench.colorCustomizations": { | ||
"activityBar.activeBackground": "#2a4d34", // Dark green for active background | ||
"activityBar.background": "#2a4d34", // Dark green for background | ||
"sash.hoverBorder": "#2a4d34", // Dark green hover border | ||
"statusBar.background": "#1b362e", // Darker green for status bar | ||
"statusBarItem.hoverBackground": "#2a4d34", // Dark green hover for status bar item | ||
"statusBarItem.remoteBackground": "#1b362e", // Darker green for remote status bar item | ||
"titleBar.activeBackground": "#1b362e", // Dark green for active title bar | ||
"titleBar.inactiveBackground": "#1b362e99", // Dark green for inactive title bar with transparency | ||
"tab.activeBorder": "#2a4d34", // Dark green active tab border | ||
"activityBarBadge.background": "#b32d4f", // Retained red badge background | ||
"activityBarBadge.foreground": "#e7e7e7", // White badge text | ||
"activityBar.foreground": "#e7e7e7", // White activity bar text | ||
"activityBar.inactiveForeground": "#e7e7e799", // Faded white for inactive activity bar | ||
"commandCenter.border": "#e7e7e799", // Light grey border for command center | ||
"statusBar.foreground": "#e7e7e7", // White text for status bar | ||
"statusBarItem.remoteForeground": "#e7e7e7", // White text for remote status bar item | ||
"titleBar.activeForeground": "#e7e7e7", // White text for active title bar | ||
"titleBar.inactiveForeground": "#e7e7e799", // Faded white for inactive title bar | ||
"statusBar.debuggingBackground": "#3a2c21", // Dark brownish-green for debugging status bar | ||
"statusBar.debuggingForeground": "#e7e7e7", // White text for debugging status bar | ||
"editorGroup.border": "#2a4d34", // Dark green for editor group border | ||
"panel.border": "#2a4d34", // Dark green for panel border | ||
"sideBar.border": "#2a4d34", // Dark green for sidebar border | ||
"statusBar.border": "#1b362e", // Dark green for status bar border | ||
"statusBar.debuggingBorder": "#3a2c21", // Dark brownish-green border for debugging status bar | ||
"titleBar.border": "#1b362e", // Dark green for title bar border | ||
"folderPathColor.custom1": "#4a6c34", // Dark olive green for custom folder path 1 | ||
"folderPathColor.custom2": "#8d5f3f", // Earthy brown for custom folder path 2 | ||
"folderPathColor.custom3": "#557e47", // Medium dark green for custom folder path 3 | ||
"folderPathColor.custom4": "#5a7d58", // Muted green for custom folder path 4 | ||
"folderPathColor.custom5": "#4c9f85" // Teal green for custom folder path 5 | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IDE-specific configurations are user-dependent and should not be included in the codebase.
{ | |
"workbench.colorCustomizations": { | |
"activityBar.activeBackground": "#2a4d34", // Dark green for active background | |
"activityBar.background": "#2a4d34", // Dark green for background | |
"sash.hoverBorder": "#2a4d34", // Dark green hover border | |
"statusBar.background": "#1b362e", // Darker green for status bar | |
"statusBarItem.hoverBackground": "#2a4d34", // Dark green hover for status bar item | |
"statusBarItem.remoteBackground": "#1b362e", // Darker green for remote status bar item | |
"titleBar.activeBackground": "#1b362e", // Dark green for active title bar | |
"titleBar.inactiveBackground": "#1b362e99", // Dark green for inactive title bar with transparency | |
"tab.activeBorder": "#2a4d34", // Dark green active tab border | |
"activityBarBadge.background": "#b32d4f", // Retained red badge background | |
"activityBarBadge.foreground": "#e7e7e7", // White badge text | |
"activityBar.foreground": "#e7e7e7", // White activity bar text | |
"activityBar.inactiveForeground": "#e7e7e799", // Faded white for inactive activity bar | |
"commandCenter.border": "#e7e7e799", // Light grey border for command center | |
"statusBar.foreground": "#e7e7e7", // White text for status bar | |
"statusBarItem.remoteForeground": "#e7e7e7", // White text for remote status bar item | |
"titleBar.activeForeground": "#e7e7e7", // White text for active title bar | |
"titleBar.inactiveForeground": "#e7e7e799", // Faded white for inactive title bar | |
"statusBar.debuggingBackground": "#3a2c21", // Dark brownish-green for debugging status bar | |
"statusBar.debuggingForeground": "#e7e7e7", // White text for debugging status bar | |
"editorGroup.border": "#2a4d34", // Dark green for editor group border | |
"panel.border": "#2a4d34", // Dark green for panel border | |
"sideBar.border": "#2a4d34", // Dark green for sidebar border | |
"statusBar.border": "#1b362e", // Dark green for status bar border | |
"statusBar.debuggingBorder": "#3a2c21", // Dark brownish-green border for debugging status bar | |
"titleBar.border": "#1b362e", // Dark green for title bar border | |
"folderPathColor.custom1": "#4a6c34", // Dark olive green for custom folder path 1 | |
"folderPathColor.custom2": "#8d5f3f", // Earthy brown for custom folder path 2 | |
"folderPathColor.custom3": "#557e47", // Medium dark green for custom folder path 3 | |
"folderPathColor.custom4": "#5a7d58", // Muted green for custom folder path 4 | |
"folderPathColor.custom5": "#4c9f85" // Teal green for custom folder path 5 | |
} | |
} |
awesome_kanban/__manifest__.py
Outdated
|
||
#Comment for creating a pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget to remove this.
estate/__init__.py
Outdated
@@ -0,0 +1,2 @@ | |||
# -*- coding: utf-8 -*- | |||
from . import models |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from . import models | |
from . import models | |
Remember to leave an empty line at the end of files. While it seems trivial, it ensures compatibility with many tools and version control systems.
estate/models/__init__.py
Outdated
from . import estate_property | ||
from . import estate_property_type | ||
from . import estate_property_tag | ||
from . import estate_property_offer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as before, missing trailing line.
estate/models/estate_property.py
Outdated
offer_ids = fields.One2many('estate.property.offer', 'property_id') | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as before, missing trailing line.
estate/models/estate_property.py
Outdated
|
||
postcode = fields.Char('Postcode') | ||
|
||
date_availability = fields.Date('Available From', copy=False, default=date.today() + timedelta(days=90)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
date_availability = fields.Date('Available From', copy=False, default=date.today() + timedelta(days=90)) | |
date_availability = fields.Date('Available From', copy=False, default=lambda: fields.Date.today() + timedelta(days=90)) |
- With the way things are now, the default date will be calculated only once at the start of the server. To make sure that it is calculated correctly every time it is need, you need to wrap the value inside a function.
- for date default values, we prefer to get today's date from
fields.Date.today()
- also, you can use
months=3
instead ofdays=90
,
estate/__manifest__.py
Outdated
# any module necessary for this one to work correctly | ||
'depends': ['base'], | ||
'application': True, | ||
'installable': True, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'installable': True, |
installable
is true by default.
estate/models/estate_property.py
Outdated
garden_orientation = fields.Selection( | ||
string='Garden Orientation', | ||
selection=[('north', 'North'), ('south', 'South'), ('east', 'East'), ('west', 'West')]) | ||
|
||
state = fields.Selection( | ||
string='Status', | ||
required=True, | ||
copy=False, | ||
default='new', | ||
selection=[('new', 'New'), ('received', 'Offer Received'), ('accepted', 'Offer Accepted'), ('sold', 'Sold'), ('cancelled', 'Cancelled')]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick 😄
garden_orientation = fields.Selection( | |
string='Garden Orientation', | |
selection=[('north', 'North'), ('south', 'South'), ('east', 'East'), ('west', 'West')]) | |
state = fields.Selection( | |
string='Status', | |
required=True, | |
copy=False, | |
default='new', | |
selection=[('new', 'New'), ('received', 'Offer Received'), ('accepted', 'Offer Accepted'), ('sold', 'Sold'), ('cancelled', 'Cancelled')]) | |
garden_orientation = fields.Selection( | |
string="Garden Orientation", | |
selection=[ | |
('north', "North"), | |
('south', "South"), | |
('east', "East"), | |
('west', "West"), | |
], | |
) | |
state = fields.Selection( | |
string="Status", | |
required=True, | |
copy=False, | |
default='new', | |
selection=[ | |
('new', "New"), | |
('received', "Offer Received"), | |
('accepted', "Offer Accepted"), | |
('sold', "Sold"), | |
('cancelled', "Cancelled"), | |
], | |
) |
<field name="living_area"/> | ||
<field name="facades"/> | ||
<separator/> | ||
<filter string="Available" name="available" domain="['|', ('state', '=', 'new'), ('state', '=', 'received')]"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could also use the in
domain operator.
<field name="name">estate.property.tag.search</field> | ||
<field name="model">estate.property.tag</field> | ||
<field name="arch" type="xml"> | ||
<search string="Search Tags"> | ||
<field name="name"/> | ||
</search> | ||
</field> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<field name="name">estate.property.tag.search</field> | |
<field name="model">estate.property.tag</field> | |
<field name="arch" type="xml"> | |
<search string="Search Tags"> | |
<field name="name"/> | |
</search> | |
</field> | |
<field name="name">estate.property.tag.search</field> | |
<field name="model">estate.property.tag</field> | |
<field name="arch" type="xml"> | |
<search string="Search Tags"> | |
<field name="name" /> | |
</search> | |
</field> |
@cemalfarukguney I didn't know about this PR and had to retrieve its information, you may have to re-approve it as I didn't see previous commands. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job. Just a few more remarks.
awesome_kanban/__manifest__.py
Outdated
{ | ||
'name': "Awesome Kanban", | ||
'summary': """ | ||
"name": "Awesome Kanban", | ||
"summary": """ | ||
Starting module for "Master the Odoo web framework, chapter 4: Customize a kanban view" | ||
""", | ||
|
||
'description': """ | ||
"description": """ | ||
Starting module for "Master the Odoo web framework, chapter 4: Customize a kanban view. | ||
""", | ||
|
||
'version': '0.1', | ||
'application': True, | ||
'category': 'Tutorials/AwesomeKanban', | ||
'installable': True, | ||
'depends': ['web', 'crm'], | ||
'data': [ | ||
'views/views.xml', | ||
"version": "0.1", | ||
"application": True, | ||
"category": "Tutorials/AwesomeKanban", | ||
"installable": True, | ||
"depends": ["web", "crm"], | ||
"data": [ | ||
"views/views.xml", | ||
], | ||
'assets': { | ||
'web.assets_backend': [ | ||
'awesome_kanban/static/src/**/*', | ||
"assets": { | ||
"web.assets_backend": [ | ||
"awesome_kanban/static/src/**/*", | ||
], | ||
}, | ||
'license': 'AGPL-3' | ||
"license": "AGPL-3", | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider avoiding formatting-only changes in a file, as they can clutter the commit history and increase the likelihood of merge conflicts.
Simple comment