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

fix: page_number for pdfs with pages not containing any text #7271

Merged
merged 7 commits into from
Mar 4, 2024

Conversation

tstadel
Copy link
Member

@tstadel tstadel commented Feb 29, 2024

Related Issues

  • fixes #issue-number

Proposed Changes:

  • correct page meta field for pdfs that contain pages without any text content

How did you test it?

Notes for the reviewer

Checklist

@tstadel tstadel requested a review from a team as a code owner February 29, 2024 16:43
@tstadel tstadel requested review from julian-risch and removed request for a team February 29, 2024 16:43
@anakin87
Copy link
Member

(Sorry to intrude...)

@vblagoje - release manager
If this is merged in the v1.25.x branch, let's make sure to cherry-pick this commit to the v1.x branch.
(otherwise, we can do the opposite)

@vblagoje
Copy link
Member

vblagoje commented Mar 1, 2024

@anakin87 sure thing

@vblagoje
Copy link
Member

vblagoje commented Mar 1, 2024

Where is the github feature to copy the PR and change the target branch when a man needs it?

@anakin87
Copy link
Member

anakin87 commented Mar 1, 2024

If you want to change the target branch, you click "edit" in the top right of this page.
image

@vblagoje
Copy link
Member

vblagoje commented Mar 1, 2024

I know that part but I want copy the PR and change the target branch, do they have that?

Copy link
Member

@julian-risch julian-risch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks good to me, although I would have appreciated some minimal test. 👍
Before merging please sync with the release manager @vblagoje
Our usual approach is to merge into the v1.x branch and then the release manager cherry picks into the v1.25.x release branch.

@tstadel
Copy link
Member Author

tstadel commented Mar 1, 2024

Change looks good to me, although I would have appreciated some minimal test. 👍 Before merging please sync with the release manager @vblagoje Our usual approach is to merge into the v1.x branch and then the release manager cherry picks into the v1.25.x release branch.

I'll add a test in a few minutes.

@tstadel tstadel changed the base branch from v1.25.x to v1.x March 2, 2024 09:12
@tstadel
Copy link
Member Author

tstadel commented Mar 2, 2024

@vblagoje I changed the target branch to v1.x. Should be good now.

@vblagoje vblagoje merged commit 7be8f1e into v1.x Mar 4, 2024
49 checks passed
@vblagoje vblagoje deleted the fix/preprocessor_page_empty_pdf_pages branch March 4, 2024 12:38
vblagoje added a commit that referenced this pull request Mar 4, 2024
* update version

* ci: fix rest-api tests (#7256)

* fix: page_number for pdfs with pages not containing any text

* Revert "fix: page_number for pdfs with pages not containing any text"

This reverts commit ae20bae.

* Reapply "fix: page_number for pdfs with pages not containing any text"

This reverts commit 18fd55b.

* add test

---------

Co-authored-by: Vladimir Blagojevic <[email protected]>
Co-authored-by: Tobias Wochinger <[email protected]>
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.

5 participants