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] Migrate hr_employee_phone_extension #112

Merged
merged 15 commits into from
Aug 12, 2015
Merged

Conversation

charbeljc
Copy link

No description provided.

@max3903 max3903 added this to the 8.0 milestone Jun 9, 2015
@max3903 max3903 mentioned this pull request Jun 9, 2015
58 tasks
@ghost
Copy link

ghost commented Jun 9, 2015

Please move the view to views/hr_employee.xml and the hr.py to models/hr_employee.py.

@ghost
Copy link

ghost commented Jun 14, 2015

@charbeljc LGTM 👍 Don't forget to add yourself as contributor.

@@ -23,7 +23,7 @@
from openerp.osv import fields, orm


class hr_employee(orm.Model):
Copy link
Member

Choose a reason for hiding this comment

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

Can you migrate to new the API? This is a trivial module, so I don't think it is too much to ask.

Copy link
Author

Choose a reason for hiding this comment

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

Hello @StefanRijnhart, done.

@StefanRijnhart
Copy link
Member

Thanks! 👍




Installation
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Just remove these sections where there is no useful content to add

@dreispt
Copy link
Sponsor Member

dreispt commented Jul 17, 2015

Some comments/suggestions on the README, but 👍

@charbeljc
Copy link
Author

@dreispt, README.rst cleaned up, PR rebased.

* internal_number
* short_number

Bugs are tracked on `GitHub Issues <https://github.com/OCA/hr/issues>`_.
Copy link
Member

Choose a reason for hiding this comment

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

Why remove Bug tracker title?

@damdam-s
Copy link
Member

update

@@ -20,4 +20,4 @@
#
##############################################################################

from . import hr
from .models import hr_employee

Choose a reason for hiding this comment

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

I am not sure, but I think it is better to set :

from . import models

Copy link
Member

Choose a reason for hiding this comment

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

yes of course

@laetitia-gangloff
Copy link

a little remark but 👍

],
'demo': [],
'test': [],
'installable': False,
'installable': True,
'auto_install': False,
'images': [],
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

images key is deprecated

@mdietrichc2c
Copy link

👍

gurneyalex added a commit that referenced this pull request Aug 12, 2015
[MIG] Migrate hr_employee_phone_extension
@gurneyalex gurneyalex merged commit 19d64fe into OCA:8.0 Aug 12, 2015
sambarros pushed a commit to sambarros/hr that referenced this pull request Jul 26, 2018
[BSSFL-214] Setup test admin password
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.

None yet