1
0
Fork
You've already forked python-barcode
0

Update codex.py #56

Open
srolin wants to merge 2 commits from srolin/master into main
pull from: srolin/master
merge into: WhyNotHugo:main
WhyNotHugo:main
WhyNotHugo:pre-commit-ci-update-config
WhyNotHugo:extensibility
WhyNotHugo:fix_sizes
WhyNotHugo:feature/longer_guard_bar
WhyNotHugo:ean-without-checksum
WhyNotHugo:readme-formatting
WhyNotHugo:cleaning
WhyNotHugo:cleanup
WhyNotHugo:michieldwitte/left-alignment-fix
srolin commented 2019年10月28日 02:29:52 +01:00 (Migrated from github.com)
Copy link

Fixes for bug #55
writer_options{"add_checksum": False} being ignored for Code39 barcodes.
Added class variable to track when the checksum has been added so it can't add twice
Moved the check for checksum to the render function
Set a default value class variable.

Fixes for bug #55 writer_options{"add_checksum": False} being ignored for Code39 barcodes. Added class variable to track when the checksum has been added so it can't add twice Moved the check for checksum to the render function Set a default value class variable.
WhyNotHugo (Migrated from github.com) reviewed 2019年10月29日 13:48:12 +01:00
WhyNotHugo (Migrated from github.com) left a comment
Copy link

Seems to make sense to me -- tho I can't see how the checksum got calculated twice. Have you figured out where that happened?

Seems to make sense to me -- tho I can't see how the checksum got calculated twice. Have you figured out where that happened?
WhyNotHugo (Migrated from github.com) commented 2019年10月29日 13:46:07 +01:00
Copy link

Moving this to the constructor seems more intuitive (since it's an instance attribute, and not a class attribute).
Also, should probably be named add_checksum, since this isn't the default, but the current value.

Moving this to the constructor seems more intuitive (since it's an instance attribute, and not a class attribute). Also, should probably be named `add_checksum`, since this isn't the default, but the current value.
WhyNotHugo (Migrated from github.com) commented 2019年10月29日 13:42:45 +01:00
Copy link

List comprehension seems redundant here.

List comprehension seems redundant here.
srolin (Migrated from github.com) reviewed 2019年10月29日 17:06:32 +01:00
srolin (Migrated from github.com) commented 2019年10月29日 17:06:32 +01:00
Copy link

Completely agree, not sure how that line got changed.

Completely agree, not sure how that line got changed.
srolin (Migrated from github.com) reviewed 2019年10月29日 17:08:52 +01:00
srolin (Migrated from github.com) commented 2019年10月29日 17:08:51 +01:00
Copy link

This was intended to be the default value that gets applied if no checksum value is passed in the options, but yes the constructor would probably be a better place.

This was intended to be the default value that gets applied if no checksum value is passed in the options, but yes the constructor would probably be a better place.
srolin (Migrated from github.com) reviewed 2019年10月29日 18:43:36 +01:00
srolin (Migrated from github.com) commented 2019年10月29日 18:43:35 +01:00
Copy link

No I didn't see an example of it being added twice, but if anyone called the render function twice then it would.
Agreed on list comprehension

No I didn't see an example of it being added twice, but if anyone called the render function twice then it would. Agreed on list comprehension
WhyNotHugo commented 2019年10月30日 10:46:09 +01:00 (Migrated from github.com)
Copy link

render doesn't seem to modify self.code. How does the checksum end up being added twice? It would seem that it's only done in the constructor (or am I missing something?).

`render` doesn't seem to modify `self.code`. How does the checksum end up being added twice? It would seem that it's only done in the constructor (or am I missing something?).
srolin commented 2019年11月01日 06:04:40 +01:00 (Migrated from github.com)
Copy link

render doesn't seem to modify self.code. How does the checksum end up being added twice? It would seem that it's only done in the constructor (or am I missing something?).

This was changed in the first commit. I moved the checksum code from the class constructor to the render function. The second commit was just to address your previous comments. If you look at the combined changes you can see the new render code.

> `render` doesn't seem to modify `self.code`. How does the checksum end up being added twice? It would seem that it's only done in the constructor (or am I missing something?). This was changed in the first commit. I moved the checksum code from the class constructor to the render function. The second commit was just to address your previous comments. If you look at the combined changes you can see the new render code.
WhyNotHugo commented 2019年11月01日 10:46:03 +01:00 (Migrated from github.com)
Copy link

Sorry, I'm not seeing it. 😓
Can you point me to the exact line where this happens? Or maybe a sample code to reproduce the issue? Or a unit test that fails for the bug?

Sorry, I'm not seeing it. :sweat: Can you point me to the exact line where this happens? Or maybe a sample code to reproduce the issue? Or a unit test that fails for the bug?
srolin commented 2019年11月05日 15:07:04 +01:00 (Migrated from github.com)
Copy link

I think perhaps I have not explained the change very well. I am traveling at the moment, but will take some time and fully document the bug, the fix, what code breaks etc. Thanks for your patience.

I think perhaps I have not explained the change very well. I am traveling at the moment, but will take some time and fully document the bug, the fix, what code breaks etc. Thanks for your patience.
WhyNotHugo commented 2020年02月25日 01:34:18 +01:00 (Migrated from github.com)
Copy link

Hey there! Do you have any update?

Hey there! Do you have any update?
anukaal (Migrated from github.com) approved these changes 2022年01月08日 18:16:34 +01:00
This pull request has changes conflicting with the target branch.
  • barcode/codex.py
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin srolin/master:srolin/master
git switch srolin/master

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main
git merge --no-ff srolin/master
git switch srolin/master
git rebase main
git switch main
git merge --ff-only srolin/master
git switch srolin/master
git rebase main
git switch main
git merge --no-ff srolin/master
git switch main
git merge --squash srolin/master
git switch main
git merge --ff-only srolin/master
git switch main
git merge srolin/master
git push origin main
Sign in to join this conversation.
No reviewers
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
WhyNotHugo/python-barcode!56
Reference in a new issue
WhyNotHugo/python-barcode
No description provided.
Delete branch "srolin/master"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?