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

ported hr_webcam to 8.0 #65

Merged
merged 14 commits into from
Jun 26, 2015
Merged

ported hr_webcam to 8.0 #65

merged 14 commits into from
Jun 26, 2015

Conversation

yoyo2k
Copy link

@yoyo2k yoyo2k commented Jan 15, 2015

  • updated __openerp__.py and hr_webcam_view.xml to reflect changes to 8.0 api
  • modified hr_webcam.css (wider camera viewport)

- updated __openerp__.py and hr_webcam_view.xml to reflect changes to 8.0 api
- modified hr_webcam.css (wider camera viewport)
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling d247a69 on yoyo2k:8.0 into 1104200 on OCA:8.0.

@yvaucher
Copy link
Member

@yoyo2k You have some pep8 issues to fix:

./hr_webcam/__init__.py:1:1: E265 block comment should start with '# '
./hr_webcam/__init__.py:8:80: E501 line too long (80 > 79 characters)
./hr_webcam/__openerp__.py:1:1: E265 block comment should start with '# '
./hr_webcam/__openerp__.py:8:80: E501 line too long (80 > 79 characters)
./hr_webcam/hr.py:1:1: F401 'fields' imported but unused
./hr_webcam/hr.py:11:20: E711 comparison to None should be 'if cond is None:'

@yoyo2k
Copy link
Author

yoyo2k commented Jan 22, 2015

Yeah.. saw those.. I just made minor adjustments to work on 8.0. The rest is unchanged. If needed I'll make the necessary changes to comply to PEP8.

@@ -1,12 +1,12 @@
# -*- coding:utf-8 -*-
#-*- coding:utf-8 -*-
Copy link
Member

Choose a reason for hiding this comment

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

Why did you changed this line?

This generate a PEP8 error:
./hr_webcam/__init__.py:1:1: E265 block comment should start with '# '

@yvaucher
Copy link
Member

@yoyo2k As you are porting it, it is the right time to fix those PEP8 issues.

BTW, some errors are due to your changes.

# it under the terms of the GNU Affero General Public License as published
# by the Free Software Foundation, either version 3 of the License, or
# it under the terms of the GNU Affero General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
Copy link
Member

Choose a reason for hiding this comment

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

Please revert this change

./hr_webcam/__init__.py:8:80: E501 line too long (80 > 79 characters)

@yoyo2k
Copy link
Author

yoyo2k commented Jan 23, 2015

Honest mistake/typo..

@coveralls
Copy link

Coverage Status

Coverage remained the same at 45.45% when pulling 4f443af on yoyo2k:8.0 into 1104200 on OCA:8.0.

@@ -0,0 +1,24 @@
from openerp.osv import fields, osv
Copy link
Member

Choose a reason for hiding this comment

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

./hr_webcam/hr.py:1:1: F401 'fields' imported but unused

@yvaucher
Copy link
Member

Could you also move description from __openerp__.py to README.rst
here is the template:
https://github.com/OCA/maintainers-tools/blob/master/template/module/README.rst

@bwrsandman
Copy link

There are conflicts in the PR, a rebase may be necessary

@max3903 max3903 modified the milestone: 8.0 Feb 14, 2015
@max3903 max3903 removed the 8.0 label Feb 14, 2015
@yoyo2k
Copy link
Author

yoyo2k commented Feb 24, 2015

sorry for the long delay.
fixed issues @yvaucher pointed out and rebased repo.

------------

* Pedro M. Baeza <pedro.baeza@serviciosbaeza.com>

Choose a reason for hiding this comment

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

Don't you want to add yourself here?

Copy link
Member

Choose a reason for hiding this comment

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

It seems that this PR mixes one of my modules, so he has to remove it, not to attribute it.

Copy link
Author

Choose a reason for hiding this comment

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

true.. because of the rebase..

Copy link
Author

Choose a reason for hiding this comment

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

Closing this pr and opening a new one could fix this.

@bwrsandman
Copy link

Travis is failing because hr hasn't been renamed in hr_webcam/__init__.py

@yoyo2k
Copy link
Author

yoyo2k commented Feb 25, 2015

and now? why is it failing?

@bwrsandman
Copy link

Not sure...

@yoyo2k
Copy link
Author

yoyo2k commented Feb 26, 2015

Ok.. more than 40 days gone by. I've done everything that was asked. On my system, it works.



class hr_employee(models.Model):
_name = 'hr.employee'
Copy link
Member

Choose a reason for hiding this comment

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

Here you wanted to remove _name and not _inherit see https://github.com/yoyo2k/hr/commit/05296ab2269cf4397a5ef25bb3c0dd0074da28a2

@yvaucher
Copy link
Member

@yoyo2k See my comment I think this is the commit which broke the module (and the tests)

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 8d55947 on yoyo2k:8.0 into * on OCA:8.0*.

@yvaucher
Copy link
Member

👍

1 similar comment
@bwrsandman
Copy link

👍

@bwrsandman
Copy link

Needs a rebase

* up_8.0:
  Add OCA as author of OCA addons
@coveralls
Copy link

Coverage Status

Coverage decreased (-12.5%) to 50.0% when pulling 365d27f on yoyo2k:8.0 into a222620 on OCA:8.0.

@yoyo2k
Copy link
Author

yoyo2k commented Mar 3, 2015

@max3903 max3903 mentioned this pull request Apr 3, 2015
58 tasks
@hbrunn
Copy link
Member

hbrunn commented May 18, 2015

@yoyo2k fix it for your own file in __openerp__.py as

    'author': "Michael Telahun Makonnen <mmakonnen@gmail.com>,"
    "Odoo Community Association (OCA)",

@mdietrichc2c
Copy link

@yoyo2k is it possible to modify your __openerp__.py as @hbrunn said? This way, we can give the go-ahead for a merge.

@jgrandguillaume
Copy link
Member

@yoyo2k Any news here ?

Thanks for the work

@bwrsandman bwrsandman merged commit 365d27f into OCA:8.0 Jun 26, 2015
bwrsandman pushed a commit that referenced this pull request Jun 26, 2015
ported hr_webcam to 8.0
@bwrsandman
Copy link

Fixed the small issue and merged manually

@aymenrahmaniisims
Copy link

just a white picture, my cam is activated but the image be white ?

sambarros pushed a commit to sambarros/hr that referenced this pull request Jul 26, 2018
[BSSFL-124] Move line credit/debit amount calculation from amount_currency
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

10 participants