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.

Sign up for GitHub

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

Jump to bottom

12.0 mig barcodes generator abstract #200

Merged
merged 18 commits into from Aug 16, 2019
Merged

12.0 mig barcodes generator abstract #200

merged 18 commits into from Aug 16, 2019

Conversation

cvinh
Copy link
Contributor

@cvinh cvinh commented Aug 13, 2019 •
edited

works out of the box in v12

legalsylvain and others added 14 commits August 12, 2019 15:41
@legalsylvain @cvinh
create new module 'barcodes_generator_abstract
fab4154
@lasley @cvinh
[MIG] barcodes_generator_abstract: Upgrade to v10
462bca1 
* Rename manifest
* Change openerp references to odoo
* Bump version
* Alphabetize imports
* pyBarcode is no longer maintained, switch to viivakoodi fork (https://bitbucket.org/whitie/python-barcode/issues/16/pypi-08-release-request#comment-33978213)
@lasley @cvinh
[10.0][IMP] barcodes_generator_abstract: Add automated generator (OCA#77
6aef4ab 
)

Add an option to allow for automatic generation of barcode when a record of mixin model is created
@lasley @cvinh
[10.0][FIX] barcodes_generator_abstract: Caching issue (OCA#103)
a4f5577 
* [FIX] barcodes_generator_abstract: Caching issue
* Make cache method return ids & move the recordset method out of cache to fix OCA#93
@oca-transbot @cvinh
OCA Transbot updated translations from Transifex
1802302
@legalsylvain @cvinh
[FIX] do not copy barcode_base during duplication
ea3cc1f
@phatnguyenuit @cvinh
[MIG] barcodes_generator_abstract: Migration to 11.0
90cd959
@pedrobaeza @cvinh
[FIX] barcodes_generator_abstract: Remove credits
3fa4b9d
@oca-travis @cvinh
[UPD] Update barcodes_generator_abstract.pot
daf0448
@oca-travis @cvinh
[UPD] Update barcodes_generator_abstract.pot
547cdc4
@oca-travis @cvinh
[UPD] Update barcodes_generator_abstract.pot
05ef126
@oca-transbot @cvinh
Update translation files
4cfc5fc 
Updated by Update PO files to match POT (msgmerge) hook in Weblate.
@oca-travis @cvinh
[UPD] Update barcodes_generator_abstract.pot
32a9044
@oca-transbot @cvinh
Update translation files
2c3d463 
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: stock-logistics-barcode-11.0/stock-logistics-barcode-11.0-barcodes_generator_abstract
Translate-URL: https://translation.odoo-community.org/projects/stock-logistics-barcode-11-0/stock-logistics-barcode-11-0-barcodes_generator_abstract/
phatnguyenuit
phatnguyenuit suggested changes Aug 13, 2019
View reviewed changes
Copy link
Member

@phatnguyenuit phatnguyenuit left a comment

Choose a reason for hiding this comment

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

Missing file requirements.txt at the top level of this repository. Refer the old one to get it.

@cvinh cvinh force-pushed the 12.0-mig-barcodes_generator_abstract branch from 47bfbe2 to 2beae2a Compare August 13, 2019 02:13
@cvinh cvinh mentioned this pull request Aug 13, 2019
Merged
1 task
@pedrobaeza pedrobaeza added this to the 12.0 milestone Aug 13, 2019
@OCA-git-bot OCA-git-bot mentioned this pull request Aug 13, 2019
Closed
14 tasks
pedrobaeza
pedrobaeza reviewed Aug 13, 2019
View reviewed changes
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

And you should check #193 for an alternative library.

barcodes_generator_abstract/models/barcode_generate_mixin.py

class BarcodeGenerateMixin(models.AbstractModel):

_name = 'barcode.generate.mixin'
Copy link
Member

Choose a reason for hiding this comment

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

Add _description for avoiding runbot warning

Copy link
Contributor Author

Choose a reason for hiding this comment

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

About the library, tell me if I should change it
About _description, I will do a force push tomorrow

Copy link
Member

Choose a reason for hiding this comment

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

@legalsylvain @yucer what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi. As said in #193, I have no point of view, regarding PIL / Pillow neither regarding the quality of the code of both librairies.
But the othe lib has more contributors. So I'm in favour to switch, if it works correctly.

@cvinh, could you test and check if all is OK ?

regards.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pedrobaeza could you help with the runbot error ? I don't see where it fails... thanks in advance

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gurneyalex could you help with the runbot error ? I don't see where it fails... thanks in advance

Copy link

Choose a reason for hiding this comment

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

@pedrobaeza sorry for the late response. I am not too much familiarized with these modules. Just noted that there were two alternate python libraries for the same feature, and the one not used had more activity.

Thanks for the feed back.

Copy link
Member

Choose a reason for hiding this comment

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

Module was already merge, so a bit late I'm afraid. Any way, we have switched the library.

@pedrobaeza
Copy link
Member

The warning is:

2019-08-13 23:03:45,710 161 WARNING openerp_test odoo.addons.base.models.ir_model: Two fields (sequence_id, sequence) of barcode.rule() have the same label: Sequence.

legalsylvain
legalsylvain approved these changes Aug 16, 2019
View reviewed changes
Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

/ocabot merge

LGTM. Thanks for porting this module.

@legalsylvain
Copy link
Contributor

/ocabot merge

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Rebased to 12.0-ocabot-merge-pr-200-by-legalsylvain-bump-no, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 7ed148f into OCA:12.0 Aug 16, 2019
OCA-git-bot added a commit that referenced this pull request Aug 16, 2019
@OCA-git-bot
Merge PR #200 into 12.0
19b2aec 
Signed-off-by legalsylvain
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 19b2aec. Thanks a lot for contributing to OCA. ❤️

PS: Don't worry if GitHub says there are unmerged commits: it is due to a rebase before merge. All commits of this PR have been merged into 12.0.

@cvinh cvinh deleted the 12.0-mig-barcodes_generator_abstract branch August 19, 2019 03:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yucer yucer yucer left review comments

@pedrobaeza pedrobaeza pedrobaeza left review comments

@legalsylvain legalsylvain legalsylvain approved these changes

@phatnguyenuit phatnguyenuit phatnguyenuit approved these changes

Assignees
No one assigned
Labels
approved merged 🎉
Projects
None yet
Milestone
12.0
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@cvinh @pedrobaeza @legalsylvain @OCA-git-bot @yucer @phatnguyenuit @lasley @oca-transbot @oca-travis