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

Update version to 0.11 #1066

Merged
merged 1 commit into from
Dec 10, 2019
Merged

Update version to 0.11 #1066

merged 1 commit into from
Dec 10, 2019

Conversation

blikblum
Copy link
Member

@blikblum blikblum commented Dec 4, 2019

What kind of change does this PR introduce?

Set package version to 0.11 and update docs accordingly

Checklist:

  • Unit Tests
  • Documentation
  • Update CHANGELOG.md
  • Ready to be merged

Will let @devongovett merge and upload a new npm release

@lcb
Copy link

lcb commented Dec 4, 2019

I am looking forward to that :)

@adamjsimon
Copy link

@devongovett Any chance you will be able to get this merged soon? I really need the infinite loop bug fix that this version would include. Thank you for your work on this great project! 💯

@adamjsimon
Copy link

@blikblum Is there anyone else who can merge and release a new npm package?

@blikblum
Copy link
Member Author

I can merge but is pointless because i cant publish it.

@adamjsimon
Copy link

I can merge but is pointless because i cant publish it.

Ah ok. Do you have any means by which to contact @devongovett to get this merged and published? I apologize for being insistent - trying to be patient but blocked by this at the moment. I really appreciate everybody's time and effort on this project and am trying to be cognizant of this project not being a day job. Trying to be patient :)

@devongovett
Copy link
Member

I'll do it when I get home from work. Will also add @blikblum on npm for future releases.

@blikblum
Copy link
Member Author

Do you have any means by which to contact @devongovett to get this merged and published?

No only here.

You can try pdfkit-next, i keep it for the unreleased fixes

@adamjsimon
Copy link

I'll do it when I get home from work. Will also add @blikblum on npm for future releases.

@devongovett Thank you so much!! 😄 👍 🎉 🎆

@adamjsimon
Copy link

Do you have any means by which to contact @devongovett to get this merged and published?

No only here.

You can try pdfkit-next, i keep it for the unreleased fixes

Thanks for that tip! I did not know about the pdfkit-next project.

@devongovett devongovett merged commit 0fc21a7 into foliojs:master Dec 10, 2019
@devongovett
Copy link
Member

@blikblum FYI the tests are failing...

@devongovett
Copy link
Member

Published anyway as v0.11.0.

btw, @blikblum looks like you already had npm publish access. 😉

$ npm owner ls pdfkit
blikblum
devongovett
diegomura
liborm85

@adamjsimon
Copy link

Published anyway as v0.11.0.

btw, @blikblum looks like you already had npm publish access. 😉

$ npm owner ls pdfkit
blikblum
devongovett
diegomura
liborm85

@devongovett Thanks for merging and publishing. When I pulled locally a couple weeks ago I noticed the integration tests were failing but I was not sure if that was expected or if that was an anomaly. If an anomaly, integration tests have been broken for at least a couple of recent commits to master.

@adamjsimon
Copy link

@devongovett Could we get a CI system setup? I don't think I have the permissions to do this but I think it would be of great use in improving code quality in merged PRs. Maybe there is one already? I don't think passing tests are enforced for merging though. Thoughts?

@blikblum
Copy link
Member Author

I noticed the integration tests were failing but I was not sure if that was expected or if that was an anomaly

The integration tests currently are not being used because the following shortcomings:

  • generated snapshots depends of the system (node version / OS). So same test produces a different output in different environments
  • due to pdf binary nature, a minimal change in output invalidates snapshots from all tests
  • The diff output is not useful at all

They played an important role in the migration from coffee to JS but now we are trying to improve the unit tests.

My idea is to replace the integration tests for visual regression tests which does not suffer from above. Until now no time.

Could we get a CI system setup? I don't think I have the permissions to do this but I think it would be of great use in improving code quality in merged PRs.

It would be a good thing. I don't know how to do but i can help merging stuff etc

@blikblum blikblum mentioned this pull request Jul 27, 2020
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants