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

Remove obsolete GD function overwrites #35560

Merged
merged 6 commits into from
Dec 4, 2022
Merged

Remove obsolete GD function overwrites #35560

merged 6 commits into from
Dec 4, 2022

Conversation

J0WI
Copy link
Contributor

@J0WI J0WI commented Dec 2, 2022 •
edited
Loading

pick from #28279, those functions are part of PHP (7.2+)

@J0WI J0WI force-pushed the rm-obsolete-gd branch 2 times, most recently from 7586189 to 065271a Compare December 2, 2022 17:17
@szaimen szaimen added the 3. to review Waiting for reviews label Dec 2, 2022
@szaimen szaimen added this to the Nextcloud 26 milestone Dec 2, 2022
@skjnldsv skjnldsv requested a review from come-nc December 2, 2022 18:10
@J0WI J0WI force-pushed the rm-obsolete-gd branch 3 times, most recently from 4dcbff2 to 185ac4b Compare December 3, 2022 14:29
@J0WI J0WI mentioned this pull request Dec 3, 2022
Draft
@szaimen szaimen added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Dec 3, 2022
@szaimen
Copy link
Contributor

szaimen commented Dec 3, 2022

Not sure if phpunit failure is related...

@szaimen
Copy link
Contributor

szaimen commented Dec 3, 2022

I think PHPunit failure is unrelated

@szaimen
Copy link
Contributor

szaimen commented Dec 3, 2022

nodb failure...

@szaimen szaimen added 2. developing Work in progress and removed 4. to release Ready to be released and/or waiting for tests to finish labels Dec 3, 2022
@szaimen
Copy link
Contributor

szaimen commented Dec 3, 2022 •
edited
Loading

image

@J0WI
Copy link
Contributor Author

J0WI commented Dec 3, 2022

getJpegQuality does not work as expected...

@szaimen
Copy link
Contributor

szaimen commented Dec 4, 2022

getJpegQuality does not work as expected...

Maybe a mock is missing somewhere?

@J0WI
Remove obsolete GD function overwrites
40166dd 
Signed-off-by: J0WI <J0WI@users.noreply.github.com>
@J0WI
Fix redundant condition
53994f6 
Signed-off-by: J0WI <J0WI@users.noreply.github.com>
@J0WI
Add type hints
1dd3afe 
Signed-off-by: J0WI <J0WI@users.noreply.github.com>
@J0WI
Allow empty mimeType
110ea0d 
Signed-off-by: J0WI <J0WI@users.noreply.github.com>
@J0WI
Cleanup test exclusions
f71b95e 
Signed-off-by: J0WI <J0WI@users.noreply.github.com>
@J0WI
Fix test image default quality
a4d0700 
Signed-off-by: J0WI <J0WI@users.noreply.github.com>
@J0WI
Copy link
Contributor Author

J0WI commented Dec 4, 2022

getAppValue is just too unreliable and assert does nothing in PHP. I started to work a bit on the typing system in #35581 but there are too many inconsistent usages of null values.
So the workaround here is to keep the null check.

@J0WI J0WI added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Dec 4, 2022
@J0WI J0WI requested a review from szaimen December 4, 2022 14:57
@szaimen
Copy link
Contributor

szaimen commented Dec 4, 2022

Thanks a lot @J0WI ! :)

@szaimen szaimen merged commit f806aa6 into nextcloud:master Dec 4, 2022
@J0WI J0WI deleted the rm-obsolete-gd branch December 4, 2022 16:28
@ChristophWurst ChristophWurst mentioned this pull request Jun 23, 2023
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skjnldsv skjnldsv skjnldsv approved these changes

@CarlSchwan CarlSchwan CarlSchwan approved these changes

@icewind1991 icewind1991 Awaiting requested review from icewind1991

@come-nc come-nc Awaiting requested review from come-nc

@szaimen szaimen Awaiting requested review from szaimen

Assignees
No one assigned
Labels
4. to release Ready to be released and/or waiting for tests to finish
Projects
None yet
Milestone
Nextcloud 26
Development

Successfully merging this pull request may close these issues.
4 participants
@J0WI @szaimen @skjnldsv @CarlSchwan