search

py-pdf / pdfly

CLI tool to extract (meta)data from PDF and manipulate PDF files
BSD 3-Clause "New" or "Revised" License
109 stars 18 forks source link

MAINT: In the cat subcommand, replace the usage of the deprecated PdfMerger by PdfWriter #34

Closed kommade closed 11 months ago

kommade commented 1 year ago

Removed deprecated PdfMerger as in #31 and replaced with PdfReader and PdfWriter. No changes made to test_case as functionality seems to be exactly the same.

Closes #31

Lucas-C commented 1 year ago

Good job @kommade!

I just approved this PR. I'll let @MartinThoma review those changes as well before merging, as he may want to perform extra checks himself to ensure the behaviour of pdfly has not changed, or maybe suggest new unit tests.

By the way, are you contributing as part of https://hacktoberfest.com @kommade?

Lucas-C commented 11 months ago

@MartinThoma : do you think that this could be merged? 🙂

MartinThoma commented 11 months ago

@kommade I'm very sorry that it took me so long to review / merge this PR :see_no_evil:

I remember that I had a quick glance, but wanted to add a few tests. Then I didn't have time for that and forgot :sweat_smile: Always feel free to gently remind me of PRs - just like Lucas did :-)

Good work with the PR! There was a small off-by-one that was easy to fix with more tests.

Your contribution will be on PyPI latest tomorrow :-)

MartinThoma commented 11 months ago

@Lucas-C Thank you for pinging me about this :hugs: And also for your help with the other issues/PRs/discussions :heart:

kommade commented 11 months ago

Hi! I actually just wanted to try contributing to public projects on github and it just happened to be hacktoberfest too! I'm glad it was accepted, no worries on the delay I wasn't too concerned anyway