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.

Sign up for GitHub

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

Jump to bottom

Detect .PDF as pdf #3590

Merged
merged 1 commit into from
Nov 29, 2021
Merged

Detect .PDF as pdf #3590

merged 1 commit into from
Nov 29, 2021

Conversation

fabian-thomas
Copy link
Contributor

@fabian-thomas fabian-thomas commented Nov 19, 2021 •
edited
Loading

Closes #3548

bhennion
bhennion reviewed Nov 20, 2021
View reviewed changes
src/util/PathUtil.cpp Outdated Show resolved Hide resolved
src/util/PathUtil.cpp Outdated Show resolved Hide resolved
personalizedrefrigerator
personalizedrefrigerator reviewed Nov 20, 2021
View reviewed changes
Copy link
Contributor

@personalizedrefrigerator personalizedrefrigerator left a comment

Choose a reason for hiding this comment

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

I haven't tested this yet, but from a quick look through the changes, this looks good to me!

@LittleHuba
Copy link
Member

@fabian-thomas please rebase to release-1.1. See https://github.com/xournalpp/xournalpp/blob/master/readme/Releases.md#branching-strategy

@fabian-thomas
Copy link
Contributor Author

That helped a little bit with understanding the different branches and tags. Although I'm not totally sure if I understood that release_helper script correctly. Is this something I should usually use/use in this case?

As I understand this I should just rebase this branch on release-1.1 so that it can be merged into that branch.

@LittleHuba
Copy link
Member

No that script is solely for us maintainers. You should never have to use it.

@LittleHuba
Copy link
Member

As I understand this I should just rebase this branch on release-1.1 so that it can be merged into that branch.

Yes exactly.

@fabian-thomas fabian-thomas changed the base branch from master to release-1.1 November 22, 2021 18:56
@fabian-thomas
Copy link
Contributor Author

@LittleHuba Have I done this correctly? I decided to cherry-pick the commits since rebasing looked like it would not make sense.

@LittleHuba
Copy link
Member

Yes cherry-picking is also a valid solution. Your changes seem to be okay.

Why did you remove the test case though?

@fabian-thomas
Copy link
Contributor Author

I accidently included it while cherry-picking the changes. It came from master. Should I include it again?

@fabian-thomas @Febbe
Fix .PDF detected as pdf
bb76213 
add hasPdfFileExt function
@Febbe Febbe merged commit 29bee34 into xournalpp:release-1.1 Nov 29, 2021
@Febbe
Copy link
Collaborator

Febbe commented Nov 29, 2021

Thanks for the PR.

@bhennion bhennion mentioned this pull request Dec 10, 2021
Merged
@rolandlo rolandlo mentioned this pull request Feb 12, 2022
Closed
8 tasks
@Technius Technius added this to the v1.1.1 milestone Feb 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bhennion bhennion bhennion left review comments

@personalizedrefrigerator personalizedrefrigerator personalizedrefrigerator left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.1.1
Development

Successfully merging this pull request may close these issues.

Unable to open PDF with capitalized extension "foo.PDF"
6 participants
@fabian-thomas @LittleHuba @Febbe @bhennion @personalizedrefrigerator @Technius