Conversation
|
Most of these diffs are formatting changes... would be nice to restrict it to the imports and leave whitespace/formatting alone. |
Greenheart
left a comment
Greenheart
left a comment
There was a problem hiding this comment.
Thanks for starting an ESM migration.
However, there are some issues that need to be addressed:
-
Like previously mentioned, a big refactor like this should preserve the formatting (including whitespace) as closely as possible to reduce noise in the diff.
-
Another big problem with the current diff is that most useful docstrings are simply removed, rather than remaining with new ESM-equivalents in the code. The docstrings should remain even in the ESM version, since they provide a lot of useful information to understand the library.
Given there are several big issues in this diff, I think this change might benefit from a fresh start in a separate branch, while keeping this branch for reference.
|
|
||
| /** | ||
| * Returns an array containing the positions of each alignment pattern. | ||
| * Each array's element represent the center point of the pattern as (x, y) coordinates | ||
| * | ||
| * Coordinates are calculated expanding the row/column coordinates returned by {@link getRowColCoords} | ||
| * and filtering out the items that overlaps with finder pattern | ||
| * | ||
| * @example | ||
| * For a Version 7 symbol {@link getRowColCoords} returns values 6, 22 and 38. | ||
| * The alignment patterns, therefore, are to be centered on (row, column) | ||
| * positions (6,22), (22,6), (22,22), (22,38), (38,22), (38,38). | ||
| * Note that the coordinates (6,6), (6,38), (38,6) are occupied by finder patterns | ||
| * and are not therefore used for alignment patterns. | ||
| * | ||
| * let pos = getPositions(7) | ||
| * // [[6,22], [22,6], [22,22], [22,38], [38,22], [38,38]] | ||
| * | ||
| * @param {Number} version QR Code version | ||
| * @return {Array} Array of coordinates | ||
| */ |
There was a problem hiding this comment.
The current diff removes useful docstrings. These should remain even in the ESM version.
| /** | ||
| * Calculate the row/column coordinates of the center module of each alignment pattern | ||
| * for the specified QR Code version. | ||
| * | ||
| * The alignment patterns are positioned symmetrically on either side of the diagonal | ||
| * running from the top left corner of the symbol to the bottom right corner. | ||
| * | ||
| * Since positions are simmetrical only half of the coordinates are returned. | ||
| * Each item of the array will represent in turn the x and y coordinate. | ||
| * @see {@link getPositions} | ||
| * | ||
| * @param {Number} version QR Code version | ||
| * @return {Array} Array of coordinate | ||
| */ |
There was a problem hiding this comment.
The current diff removes useful docstrings. These should remain even in the ESM version.
| * Returns the number of error correction block that the QR Code should contain | ||
| * for the specified version and error correction level. | ||
| * | ||
| * @param {Number} version QR Code version | ||
| * @param {Number} errorCorrectionLevel Error correction level | ||
| * @return {Number} Number of error correction blocks | ||
| */ |
There was a problem hiding this comment.
The current diff removes useful docstrings. These should remain even in the ESM version.
| /** | ||
| * Returns the number of error correction codewords to use for the specified | ||
| * version and error correction level. | ||
| * | ||
| * @param {Number} version QR Code version | ||
| * @param {Number} errorCorrectionLevel Error correction level | ||
| * @return {Number} Number of error correction codewords | ||
| */ |
There was a problem hiding this comment.
The current diff removes useful docstrings. These should remain even in the ESM version.
|
|
||
| /** | ||
| * Returns an array containing the positions of each finder pattern. | ||
| * Each array's element represent the top-left point of the pattern as (x, y) coordinates | ||
| * | ||
| * @param {Number} version QR Code version | ||
| * @return {Array} Array of coordinates | ||
| */ |
There was a problem hiding this comment.
The current diff removes useful docstrings. These should remain even in the ESM version.
| /** | ||
| * Precompute the log and anti-log tables for faster computation later | ||
| * | ||
| * For each possible value in the galois field 2^8, we will pre-compute | ||
| * the logarithm and anti-logarithm (exponential) of this value | ||
| * | ||
| * ref {@link https://en.wikiversity.org/wiki/Reed%E2%80%93Solomon_codes_for_coders#Introduction_to_mathematical_fields} | ||
| */ |
There was a problem hiding this comment.
The current diff removes useful docstrings. These should remain even in the ESM version.
|
|
||
| /** | ||
| * Returns log value of n inside Galois Field | ||
| * | ||
| * @param {Number} n | ||
| * @return {Number} | ||
| */ |
There was a problem hiding this comment.
The current diff removes useful docstrings. These should remain even in the ESM version.
|
|
||
| /** | ||
| * Returns anti-log value of n inside Galois Field | ||
| * | ||
| * @param {Number} n | ||
| * @return {Number} | ||
| */ | ||
| exports.exp = function exp (n) { |
There was a problem hiding this comment.
The current diff removes useful docstrings. These should remain even in the ESM version.
|
|
||
| /** | ||
| * Multiplies two number inside Galois Field | ||
| * | ||
| * @param {Number} x | ||
| * @param {Number} y | ||
| * @return {Number} | ||
| */ | ||
| exports.mul = function mul (x, y) { |
There was a problem hiding this comment.
The current diff removes useful docstrings. These should remain even in the ESM version.
| /** | ||
| * Data mask pattern reference | ||
| * @type {Object} | ||
| */ | ||
| exports.Patterns = { |
There was a problem hiding this comment.
The current diff removes useful docstrings. These should remain even in the ESM version.
3 participants
No description provided.