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

Proposed fix for Issue #836 - height and width parameters ignored when barcode_format is DATA_MATRIX #933

Merged
merged 3 commits into from Dec 28, 2017

Conversation

Projects
None yet
4 participants
@kent92
Contributor

kent92 commented Dec 27, 2017

Hi @srowen , was wondering if you'd like to go over my proposed fix for this issue. Tried it locally and it works, passes unit tests, etc. Let me know if further changes are needed and i will follow through accordingly.

Thanks!

Kent Avasarala Kent Avasarala
Proposed fix for Issue #836 - height and width parameters ignored whe…
…n barcode_format is DATA_MATRIX

:wq
wq
@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Dec 27, 2017

Codecov Report

Merging #933 into master will increase coverage by <.01%.
The diff coverage is 89.47%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #933      +/-   ##
============================================
+ Coverage     75.31%   75.32%   +<.01%     
- Complexity     4159     4160       +1     
============================================
  Files           252      252              
  Lines         14011    14020       +9     
  Branches       2877     2878       +1     
============================================
+ Hits          10552    10560       +8     
  Misses         2645     2645              
- Partials        814      815       +1
Impacted Files Coverage Δ Complexity Δ
.../com/google/zxing/datamatrix/DataMatrixWriter.java 82.43% <89.47%> (+0.89%) 19 <5> (+1) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6e04f9b...ecfa7f1. Read the comment docs.

codecov-io commented Dec 27, 2017

Codecov Report

Merging #933 into master will increase coverage by <.01%.
The diff coverage is 89.47%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #933      +/-   ##
============================================
+ Coverage     75.31%   75.32%   +<.01%     
- Complexity     4159     4160       +1     
============================================
  Files           252      252              
  Lines         14011    14020       +9     
  Branches       2877     2878       +1     
============================================
+ Hits          10552    10560       +8     
  Misses         2645     2645              
- Partials        814      815       +1
Impacted Files Coverage Δ Complexity Δ
.../com/google/zxing/datamatrix/DataMatrixWriter.java 82.43% <89.47%> (+0.89%) 19 <5> (+1) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6e04f9b...ecfa7f1. Read the comment docs.

@axxel

This comment has been minimized.

Show comment
Hide comment
@axxel

axxel Dec 27, 2017

My 2 cents as an outsider: why not refactor the QRCode + AZtech + DataMatrix writers so they all use the same simple scaling/inflating code instead of adding yet another copy of the functionality? (I'm biased, of course: #836 (comment) ;))

axxel commented Dec 27, 2017

My 2 cents as an outsider: why not refactor the QRCode + AZtech + DataMatrix writers so they all use the same simple scaling/inflating code instead of adding yet another copy of the functionality? (I'm biased, of course: #836 (comment) ;))

Show outdated Hide outdated core/src/main/java/com/google/zxing/datamatrix/DataMatrixWriter.java
@@ -159,17 +160,33 @@ private static BitMatrix encodeLowLevel(DefaultPlacement placement, SymbolInfo s
* @param matrix The input matrix.
* @return The output matrix.
*/
private static BitMatrix convertByteMatrixToBitMatrix(ByteMatrix matrix) {
int matrixWidgth = matrix.getWidth();
private static BitMatrix convertByteMatrixToBitMatrix(ByteMatrix matrix, int reqWidth, int reqHeight) {

This comment has been minimized.

@srowen

srowen Dec 27, 2017

Contributor

It's worth a little javadoc to clarify what these two args are, especially as they're abbreviated

@srowen

srowen Dec 27, 2017

Contributor

It's worth a little javadoc to clarify what these two args are, especially as they're abbreviated

Show outdated Hide outdated core/src/main/java/com/google/zxing/datamatrix/DataMatrixWriter.java

Kent Avasarala added some commits Dec 27, 2017

@kent92

This comment has been minimized.

Show comment
Hide comment
@kent92

kent92 Dec 28, 2017

Contributor

@srowen added the changes you've suggested. Let me know if there's more. Thanks

Contributor

kent92 commented Dec 28, 2017

@srowen added the changes you've suggested. Let me know if there's more. Thanks

@srowen srowen merged commit 1861cf4 into zxing:master Dec 28, 2017

4 checks passed

Codacy/PR Quality Review Good work! A positive pull request.
Details
codecov/patch 89.47% of diff hit (target 75.31%)
Details
codecov/project 75.32% (+<.01%) compared to 6e04f9b
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

antimony added a commit to antimony/zxing that referenced this pull request Oct 1, 2018

Proposed fix for Issue zxing#836 - height and width parameters ignore…
…d when barcode_format is DATA_MATRIX (zxing#933)

* Proposed fix for Issue zxing#836 - height and width parameters ignored when barcode_format is DATA_MATRIX

* Adding requested changes for PR zxing#933

* Wording change in Javadoc to make the description more sensible
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment