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

[MIG] project_gtd: migration 8.0 > 10.0 #308

Merged
merged 1 commit into from Jun 22, 2018

Conversation

mmequignon
Copy link
Member

No description provided.

@oca-clabot
Copy link

Hey @mmequignon, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/website.cla
Here is a list of the users:

Appreciation of efforts,
OCA CLAbot

@sebalix
Copy link

sebalix commented Aug 21, 2017

Note: @mmequignon is covered by the ABF OSIELL CLA.

@gurneyalex
Copy link
Member

@sebalix fixed the CLA issue

'Context', size=64, required=True, translate=True)
sequence = fields.Integer(
'Sequence',
help=("Gives the sequence order when displaying a list of contexts."),
Copy link
Member

Choose a reason for hiding this comment

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

if the line is short enough, you can get rid of the ( )

@gurneyalex
Copy link
Member

This looks really nice.

@gurneyalex gurneyalex added this to the 10.0 milestone Oct 2, 2017
@oca-clabot
Copy link

Hey @mmequignon,
We acknowledge that the following users have signed our Contributor License Agreement:

Appreciation of efforts,
OCA CLAbot

Copy link
Member

@Jerther Jerther left a comment

Choose a reason for hiding this comment

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

I made some improvement suggestions. Nothing critical really.

performing those tasks.
""",
'author': "OpenERP SA,Odoo Community Association (OCA)",
'author': "OpenERP SA, Odoo Community Association (OCA), ABF OSIELL",
Copy link
Member

Choose a reason for hiding this comment

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

OpenERP --> Odoo

def fields_view_get(self, view_id=None, view_type='form', toolbar=False,
submenu=False):
res = super(ProjectTask, self).fields_view_get(
view_id, view_type, toolbar=toolbar, submenu=submenu)
Copy link
Member

Choose a reason for hiding this comment

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

Arguments should be simplified with *args and **kwargs

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

@Jerther sorry?

Copy link
Member

Choose a reason for hiding this comment

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

Since all arguments are solely used in the super call, I think it'd be safer and cleaner to write:

def fields_view_get(self, *args, **kwargs):
        res = super(ProjectTask, self).fields_view_get(*args, **kwargs)

# Copyright 2017 ABF OSIELL <http://osiell.com>
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).

from odoo import fields, models, api, tools
Copy link
Member

Choose a reason for hiding this comment

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

imported lines should be alphabetically sorted


from openerp.osv import fields, osv
from openerp.tools.translate import _
from odoo import fields, models, api, exceptions
Copy link
Member

Choose a reason for hiding this comment

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

imported lines should be alphabetically sorted


from openerp.osv import fields, osv
from odoo import fields, models, api
Copy link
Member

Choose a reason for hiding this comment

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

imported lines should be alphabetically sorted

Copy link
Sponsor Member

@dreispt dreispt left a comment

Choose a reason for hiding this comment

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

Made a few comment to fix.

_order = "sequence, name"

name = fields.Char(
'Context', size=64, required=True, translate=True)
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

It is current practice to remove the size attribute from Chars

def fields_view_get(self, view_id=None, view_type='form', toolbar=False,
submenu=False):
res = super(ProjectTask, self).fields_view_get(
view_id, view_type, toolbar=toolbar, submenu=submenu)
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

@Jerther sorry?

<record model="ir.ui.view" id="view_gtd_context_tree">
<field name="name">project.gtd.context.tree</field>
<field name="model">project.gtd.context</field>
<field name="arch" type="xml">
<tree string="Context">
<tree name="Context">
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Isnt't "string" correct? Anyway, i don't think these attributes are really useful.

@@ -16,7 +15,7 @@
<field name="name">project.gtd.context.form</field>
<field name="model">project.gtd.context</field>
<field name="arch" type="xml">
<form string="Context" version="7.0">
<form name="Context" version="7.0">
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

I believe you can remove the "7.0"

self.pool.get('project.task').write(
cr, uid, data[0]['task_ids'],
{'timebox_id': data[0]['timebox_to_id'][0]})
self.task_ids.write({'timebox_id': self.timebox_to_id.id})
return {'type': 'ir.actions.act_window_close'}

# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Please remove

@mmequignon
Copy link
Member Author

It seems that tests are failing because of project_closing module.

Travis traceback
(most recent call last):
File "/home/travis/build/OCA/project/project_closing/tests/test_project.py", line 40, in test_analytic_toggle_active
self.assertFalse(self._analytic1.active)
AssertionError: True is not false

I don't know what to do about this.

@mmequignon
Copy link
Member Author

mmequignon commented Feb 15, 2018

Since project_closing have been removed from project, tests are now ok.
Is there anything else to fix ?

@sebalix
Copy link

sebalix commented Feb 16, 2018

Squash your commits please.

@mmequignon
Copy link
Member Author

Yes, that's done, thank you !

@pedrobaeza pedrobaeza merged commit 9ba642c into OCA:10.0 Jun 22, 2018
@pedrobaeza pedrobaeza mentioned this pull request Jun 22, 2018
27 tasks
@SalahAdDin
Copy link
Contributor

It seems very nice to 11.0.

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

Successfully merging this pull request may close these issues.

None yet

10 participants