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

feat: support for processing of multiple templates via CLI and API (#158) #201

Closed
wants to merge 5 commits into from
Closed

feat: support for processing of multiple templates via CLI and API (#158) #201

wants to merge 5 commits into from

Conversation

m-jung
Copy link

@m-jung m-jung commented Aug 10, 2020

m-jung added 4 commits August 10, 2020 16:22
- Feat: Support for processing of multiple templates via CLI and API …
a399338 
…(see [README.md](README.md)).

- Fixed: Where defining less font-formats using `formats` in an config-file wouldn't lead to expected result.
- Fixed: _"file not found"_ error if the parent output directory for fonts or templates didn't exist.
- Fixed: webfont.config.js was wrongly detected as test-suite by Jest…
23504ec 
…. '__test__/.*' no longer included in jest.testRegex to avoid this.

- Fixed: Mistakenly stripped trailing whitespaces, which caused tests to fail.
- Fixed sporadic missing woff2 output.
b75a81d
- Fixed: Tests / Snapshots
ce00487
jimmyandrade
jimmyandrade reviewed Sep 29, 2020
View reviewed changes
Copy link
Collaborator

@jimmyandrade jimmyandrade left a comment

Choose a reason for hiding this comment

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

@m-jung because of some changes we made to the main branch, there were some conflicts between your PR and our code. Could you help us to verify these issues so that we can integrate? Thank you.

@jimmyandrade jimmyandrade mentioned this pull request Sep 29, 2020
Open
@jimmyandrade jimmyandrade changed the title Support for processing of multiple templates via CLI and API #158 feat: support for processing of multiple templates via CLI and API (#158) Sep 29, 2020
@jimmyandrade jimmyandrade linked an issue Sep 29, 2020 that may be closed by this pull request
feat: multiple templates #158
Open
@jimmyandrade jimmyandrade linked an issue Oct 23, 2020 that may be closed by this pull request
template should accept an array #279
Open
Merge branch 'master' into multiple-templates-issue-158
755f6bb 
changed: resolved conflicts after merge from master.
changed: Magic strings 'md5' and 'hex' to constants.
fixed: Hash was based on SVG-Input only, leading to identical result-hashes while result was different.
workaround: Indeterministic results for the last byte for subsequent calls to wawoff2.compress() with equal input.
added: Hash for each resulting font format.
added: JSDoc
fixed: CLI for -formats / -r for multiple values now returns an string[].
  Code from master-branch relied on both, string and Array, having an .includes()-Method, while expected type was an Array.
changed: template option no longer accepts comma-separated template names / files / options, instead it expects an actual Array.
changed: disabled husky:pre-commit since prettier is not working as expected
@m-jung
Copy link
Author

m-jung commented Nov 26, 2020

I've merged and updated my branch with your recent changes from master.
However, some problems remain, that i've got currently no time to tackle.

  • In my branch i adopted your hash-mechanism, but rather than creating the hash based on the input, it's based on the output.
    However there seems to be an problem in the woff2 lib, which creates marginally different results (the last byte is different) for the exact same input buffer, for subsequent tests.
    I was not able to reproduce the behavior in an test that isolated the call to wawoff2.compress() a hundret times using the same input. Debugging on the other hand showed clearly, that different outputs results in the internal call of compress_wrap(inputPtr, inputSize, outputPtr)
  • The Husky pre-commit hook failed with no further usefull information, so is was forced to disable it in order to commit.
    The output i've got is equal to the Travius CI build: https://travis-ci.org/github/itgalaxy/webfont/jobs/746005346

Feel free to review.
If you are concerned about linting or prettifying i'd appreciate if you could point me in the right direction to make CI builds run successfully again.

@jimmyandrade jimmyandrade added this to the next milestone Nov 26, 2020
@jimmyandrade
Copy link
Collaborator

The codebase has changed considerably since the last commit. Could you check it out?

@m-jung
Copy link
Author

m-jung commented Apr 15, 2021

I'm afraid I'll have to politely decline your request.

@m-jung m-jung closed this Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jimmyandrade jimmyandrade jimmyandrade left review comments

@alexander-akait alexander-akait Awaiting requested review from alexander-akait

Assignees

@m-jung m-jung

Labels
enhancement help wanted
Projects
None yet
Milestone
next
Development

Successfully merging this pull request may close these issues.

template should accept an array feat: multiple templates
2 participants
@m-jung @jimmyandrade