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

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

Merged
merged 8 commits into from
Dec 16, 2023

Conversation

kommade
Copy link
Contributor

@kommade kommade commented Oct 14, 2023

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
Copy link
Member

Lucas-C commented Oct 16, 2023

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
Copy link
Member

Lucas-C commented Dec 15, 2023

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

@Lucas-C Lucas-C changed the title Resolves #31 Resolves #31 - In the cat subcommand, replace the usage of the deprecated PdfMerger class from pypdf Dec 15, 2023
@MartinThoma MartinThoma changed the title Resolves #31 - In the cat subcommand, replace the usage of the deprecated PdfMerger class from pypdf MAINT: In the cat subcommand, replace the usage of the deprecated PdfMerger by PdfWriter Dec 16, 2023
pdfly/cat.py Outdated Show resolved Hide resolved
@MartinThoma MartinThoma merged commit f31111c into py-pdf:main Dec 16, 2023
6 checks passed
@MartinThoma
Copy link
Member

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

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 😅
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
Copy link
Member

@Lucas-C Thank you for pinging me about this 🤗 And also for your help with the other issues/PRs/discussions ❤️

MartinThoma added a commit that referenced this pull request Dec 17, 2023
## What's new

### New Features (ENH)
-  Add x2pdf command (#25) by @MartinThoma

### Bug Fixes (BUG)
-  boxes are floats, not int by @MartinThoma
-  Add missing fpdf2 dependency (#29) by @MartinThoma

### Documentation (DOC)
-  cat command by @MartinThoma
-  More examples for the cat subcommand by @MartinThoma
-  Add cat subcommand by @MartinThoma
-  Link to readthedocs by @MartinThoma
-  Add project governance file by @MartinThoma
-  Move readthedocs config file to root by @MartinThoma
-  Add docs (#24) by @MartinThoma

### Developer Experience (DEV)
-  Checkout sample-files in CI (#30) by @MartinThoma
-  Let dependabot update Github Actions by @MartinThoma
-  Add action for automatic releases by @MartinThoma

### Maintenance (MAINT)
-  Update dependencies (#42) by @MartinThoma
-  In the cat subcommand, replace the usage of the deprecated PdfMerger by PdfWriter (#34) by @kommade
-  Update .pre-commit-config.yaml by @MartinThoma
-  Adjust x2pdf syntax by @MartinThoma

### Testing (TST)
-  cat with two files (#41) by @MartinThoma
-  Test cat command with more parameters + validate result (#40) by @MartinThoma
-  Adding unit tests (#28) by @Lucas-C

[Full Changelog](0.2.14...0.3.0)
@kommade
Copy link
Contributor Author

kommade commented Dec 20, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

In the cat subcommand, replace the usage of the deprecated PdfMerger class from pypdf
3 participants