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.

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

migrate away from the fork of foliojs packages #2493

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AnthonyLenglet
Copy link

Closes #2492

npm run test and npm run build both worked, it looks like some linter changes also made their way into the PR, hopefully that's still fine

@liborm85
Copy link
Collaborator

Contributing guide is available here: CONTRIBUTING.md

@AnthonyLenglet AnthonyLenglet force-pushed the feature/folio-fork-migration branch from 5e90ff4 to 05f7121 Compare October 25, 2022 12:10
@AnthonyLenglet
Copy link
Author

removed the build folder from the PR !

@liborm85
Copy link
Collaborator

CONTRIBUTING.md:
... Do not make code style changes unless it is related to the pull request.

@AnthonyLenglet AnthonyLenglet force-pushed the feature/folio-fork-migration branch from 05f7121 to 8efdd74 Compare October 25, 2022 12:56
@AnthonyLenglet
Copy link
Author

is npm run mocha recommended over npm run test then ? it seems this calls eslint, causing all of those changes 🤔

lint changes removed in the meantime though !

@AnthonyLenglet AnthonyLenglet force-pushed the feature/folio-fork-migration branch from 8efdd74 to 0bec493 Compare October 25, 2022 13:13
@liborm85
Copy link
Collaborator

eslint does not make any changes in code. This changed your IDE.

Back to the topic... I don't know if it will be that simple, because in PR foliojs/pdfkit#1367 for new fonkit version require changes in pdfkit.

@TanushreeB2611
Copy link

Can this PR be merged ? We are blocked due to this.

@liborm85
Copy link
Collaborator

Explained in my previous post.

@djgerhab
Copy link

djgerhab commented Nov 6, 2023

@AnthonyLenglet Could you please Merge? This is also a blocker for my project as it addresses a critical vulnerability in crypto-js dependency

@AnthonyLenglet
Copy link
Author

I do not have the ability to merge unfortunately, this has been a blocker to me as well, so I ended up moving to librairies that didn't have this package as a dependency instead in the meantime

@TanushreeB2611
Copy link

AnthonyLenglet: Which alternate library did you move to that doesn't have this package as dependency ?

@AnthonyLenglet
Copy link
Author

AnthonyLenglet commented Nov 9, 2023

AnthonyLenglet: Which alternate library did you move to that doesn't have this package as dependency ?

I'd have to get back to the project that was affected by this, but our problem came from compodoc, that used this library

though this library has been updated a few hours ago as of writing, which I'd have to test, but it could be a fix

Edit: pdfmake got updated, but linebreak is still the problematic version, I guess you might be able to install this branch specifically ? not sure

@spicybackend spicybackend mentioned this pull request Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants