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

[10.0] Migrate printer_zpl2 module #81

Merged
merged 2 commits into from
May 11, 2017
Merged

[10.0] Migrate printer_zpl2 module #81

merged 2 commits into from
May 11, 2017

Conversation

sla-subteno-it
Copy link

No description provided.

@lasley lasley mentioned this pull request Apr 5, 2017
Merged
@lasley
Copy link
Contributor

lasley commented Apr 5, 2017

Thanks for the contribution @alnslang. #68 already had the base_report_to_printer upgraded, so I went ahead and merged it. Can you please rebase onto the current 10.0 so that this PR becomes exclusive to printer_zpl2?

@lasley lasley added this to the 10.0 milestone Apr 5, 2017
@sla-subteno-it sla-subteno-it force-pushed the 10.0-mig-printer_zpl2 branch from 2c68018 to 9467abf Compare April 6, 2017 19:15
@sla-subteno-it
Copy link
Author

@lasley I rebase my branch and remove my modifications about base_report_to_printer. Now, my PR becomes exclusive to printer_zpl2.

@lasley
Copy link
Contributor

lasley commented Apr 6, 2017

@alnslang - Looks like tests are failing due to a missing dependency. Please create a requirements.txt in the root directory & add zpl2 as a line.

Looks like this is done incorrectly on the other branches; I'll submit a PR to fix those.

@sla-subteno-it
Copy link
Author

Thanks @lasley for your help. I just adding requirements.txt in root directory

@sla-subteno-it sla-subteno-it force-pushed the 10.0-mig-printer_zpl2 branch from 567a861 to b00ee41 Compare April 6, 2017 19:44
@sla-subteno-it sla-subteno-it changed the title [10.0] Migrate printer_zpl2 and base_report_to_printer modules [10.0] Migrate printer_zpl2 module Apr 13, 2017
[ADD] Add printer_zpl2 module (OCA#66)
d39d4c4 
* [FIX] printer_tray: Allow to call print_option with no report

* [ADD] Add printer_zpl2 module
@Garamotte Garamotte force-pushed the 10.0-mig-printer_zpl2 branch from b00ee41 to 6873ed9 Compare April 13, 2017 09:39
@Garamotte Garamotte mentioned this pull request Apr 13, 2017
Closed
5 tasks
@blutecsolutions
Copy link
Contributor

Greetings !
What's the status of this migration ?
Thanks & we appreciate all efforts.

@Garamotte
Copy link

Garamotte commented Apr 19, 2017 •
edited
Loading

Hi @blutecsolutions
The migration should be finished, waiting for reviews to be approved (or fixed).

lasley
lasley approved these changes Apr 19, 2017
View reviewed changes
Copy link
Contributor

@lasley lasley left a comment

Choose a reason for hiding this comment

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

Few comments, nothing major

printer_zpl2/__manifest__.py Outdated

{
'name': 'Printer ZPL II',
'version': '9.0.1.0.0',
Copy link
Contributor

Choose a reason for hiding this comment

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

10.0.1.0.0

Copy link

Choose a reason for hiding this comment

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

This was already changed, I don't know how you commented on that line...

printer_zpl2/__manifest__.py Outdated
'author': 'SYLEAM, Odoo Community Association (OCA)',
'website': 'http://www.syleam.fr/',
'license': 'AGPL-3',
'external_dependancies': {
Copy link
Contributor

Choose a reason for hiding this comment

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

s/external_dependancies/external_dependencies

@Garamotte Garamotte force-pushed the 10.0-mig-printer_zpl2 branch from 6873ed9 to f623f23 Compare April 20, 2017 07:21
@Garamotte
Copy link

Typo fixed, good catch @lasley :)

@lasley
Copy link
Contributor

lasley commented Apr 26, 2017

Instead of just waiting, you could provide the second review we need to merge it.

lasley
lasley reviewed Apr 26, 2017
View reviewed changes
printer_zpl2/views/printing_label_zpl2.xml Outdated
License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
-->
<odoo>
<menuitem id="base_report_to_printer.printing_menu_label" parent="base_report_to_printer.printing_menu" name="Labels" sequence="50"/>
Copy link
Contributor

@lasley lasley Apr 26, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Please remove the base_report_to_printer. from this ID in order to clear up the Runbot warning

id="printing_menu_label"

Copy link
Author

Choose a reason for hiding this comment

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

base_report_to_printer. removed.

Thanks

Copy link

Choose a reason for hiding this comment

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

The goal was to "provide" a unique menu for labels.
If another module wants to provide labels, it will create a new menu (in this case, this "Labels" menu is useless), or need to depend on printer_zpl2...

How could we achieve this ? Add the "Labels" menu in the base_report_to_printer module instead ?

@sla-subteno-it sla-subteno-it force-pushed the 10.0-mig-printer_zpl2 branch from f623f23 to a7700db Compare April 26, 2017 22:59
@blutecsolutions
Copy link
Contributor

Can we download this module ?

elicoidal
elicoidal approved these changes May 1, 2017
View reviewed changes
Copy link

@elicoidal elicoidal left a comment

Choose a reason for hiding this comment

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

small details

printer_zpl2/models/printing_label_zpl2.py Outdated
help='Origin point of the contents in the label, Y coordinate.')
width = fields.Integer(
required=True, default=480,
help='With of the label, will be set on the printer before printing.')
Copy link

Choose a reason for hiding this comment

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

Width

printer_zpl2/models/printing_label_zpl2_component.py
], required=True, default=zpl2.ORIENTATION_NORMAL,
help='Orientation of the barcode.')
check_digits = fields.Boolean(
help='Check if you want to compute and print the check digit.')
Copy link

Choose a reason for hiding this comment

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

check digit or check sum?

Copy link

Choose a reason for hiding this comment

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

It's called check digit in the documentation : https://www.zebra.com/content/dam/zebra/manuals/en-us/software/zpl-zbi2-pm-en.pdf (page 1299).

Copy link

Choose a reason for hiding this comment

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

👍

@Garamotte Garamotte force-pushed the 10.0-mig-printer_zpl2 branch from a7700db to 0328f57 Compare May 1, 2017 08:22
@Garamotte
Copy link

@elicoidal Thanks, typo fixed.
I also removed the "Labels" menu, that became useless since last change asked by @lasley and renamed the "ZPL II" menu to "ZPL II Labels".

@elicoidal
Copy link

@sylvain-garancher Excellent contribution :)

@Garamotte
Copy link

@lasley @pedrobaeza This PR is ready to merge.

@lasley lasley merged commit bbb8ca5 into OCA:10.0 May 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elicoidal elicoidal elicoidal approved these changes

@lasley lasley lasley approved these changes

@Garamotte Garamotte Garamotte left review comments

Assignees
No one assigned
Labels
needs review
Projects
None yet
Milestone
10.0
Development

Successfully merging this pull request may close these issues.
5 participants
@sla-subteno-it @lasley @blutecsolutions @Garamotte @elicoidal