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

Allow --sidecar along --pages #735

Merged
merged 1 commit into from
Feb 20, 2021
Merged

Conversation

dimakuz
Copy link
Contributor

@dimakuz dimakuz commented Feb 17, 2021

After a few short checks it looks like --sidecar and --pages can work together and produce reasonable results.
This PR relaxes this constraint and prints skipped pages as ranges.

@jbarlow83
Copy link
Collaborator

Thank you.

Because this contribution exceeds 10 lines of code, which for some reason is the magic number:

  • Is this contribution your own work and not someone else's?
  • Do you agree to allow your contribution to be redistributed under the project's current license (MPL 2.0) or any open source license the project may adopt in the future?
  • Do you have the legal authority to grant this permission? (e.g. you did not do work under a contract where an employer or client may have legal rights)

If you answer yes to all, we can proceed.

Also, would you mind adding a quick unit test to verify that enumerate_compress_ranges works as designed?

@dimakuz
Copy link
Contributor Author

dimakuz commented Feb 18, 2021

Hi @jbarlow83,
Thanks for the quick response!
The answer to all questions is yes. And sure, I'll add some tests later on today. Where would they ideally go? I see that test_pipeline.py references _pipeline module but I can put them wherever you prefer.

@jbarlow83
Copy link
Collaborator

jbarlow83 commented Feb 18, 2021 via email

@dimakuz
Copy link
Contributor Author

dimakuz commented Feb 19, 2021

@jbarlow83 I've added the test cases but CI seems to have run into some issue while running the tests.

@jbarlow83
Copy link
Collaborator

jbarlow83 commented Feb 19, 2021 via email

@jbarlow83 jbarlow83 merged commit 5e2206b into ocrmypdf:master Feb 20, 2021
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.

2 participants