fix(icons-to-woff.js): enforce deterministic output #64
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kaidjohnson
commented
Jan 5, 2022
| @@ -21,10 +21,7 @@ module.exports = function createIconFont (fs, icons, options) { | |||
| name: options.name, | |||
| normalize: true, | |||
| fontHeight: options.enforcedSvgHeight ? options.enforcedSvgHeight : undefined, | |||
| log: function () {}, | |||
| error: /** @param {any} err */function (err) { | |||
There was a problem hiding this comment.
The error property does not appear to be a supported option in svgicons2svgfont. I wasn't able to find a valid code path in that project that would call this function, so to support 100% test coverage, I opted to drop it here. The .on('error') callback on the stream below is sufficient for handling all errors raised by svgicons2svgfont.
Ensure builds with the same set of inputs results in an identical font hash for deterministic builds. re jantimon#53
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ensure builds with the same set of inputs results in an identical font hash for deterministic builds.
Improves solution proposed in #53
Notes:
icons-to-woff.jsto verify this fix actually worked. Needed to write a few additional tests to ensure 100% coverage of that file.erroroption passed toSvgicons2svgfontwas removed because it's not a supported property of that project. This issue was highlighted when unit test coverage was added toicons-to-woff.jsand the line coverage could not be met.