Skip to content

Conversation

@Altahrim
Copy link
Collaborator

@Altahrim Altahrim commented Jul 8, 2025

@Altahrim Altahrim requested review from come-nc and skjnldsv July 8, 2025 09:59
@Altahrim Altahrim self-assigned this Jul 8, 2025
@Altahrim Altahrim force-pushed the feat/allow_several_downloads_urls branch 2 times, most recently from 55e9a69 to 8ae7d3a Compare July 8, 2025 10:09
Copy link
Collaborator

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

I did not test but code looks good.
Can you add a comment for the regex?

* Check if PHP is able to decompress archive format
*/
private function isAbleToDecompress(string $ext): bool {
// Only zip is supported for now
Copy link
Collaborator

Choose a reason for hiding this comment

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

🤔 Smells like over-engineering, but ok.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At first, I wanted to support decompression for bz2 too, but it's not as straightforward as ZIP so I dropped it.
There is also another issue because we only have the signature for ZIP files.

Signed-off-by: Benjamin Gaussorgues <benjamin.gaussorgues@nextcloud.com>
@Altahrim Altahrim force-pushed the feat/allow_several_downloads_urls branch from 8ae7d3a to b8d50f6 Compare July 8, 2025 12:25
@Altahrim Altahrim marked this pull request as ready for review July 8, 2025 12:38
@Altahrim Altahrim merged commit b7afd33 into master Jul 9, 2025
21 checks passed
@Altahrim Altahrim deleted the feat/allow_several_downloads_urls branch July 9, 2025 08:40
@Altahrim
Copy link
Collaborator Author

/backport to stable31

@Altahrim
Copy link
Collaborator Author

/backport to stable30

}


use CurlHandle;
Copy link
Member

Choose a reason for hiding this comment

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

I believe this is unnecessary. Saw a report on the forum:

Warning: The use statement with non-compound name ‘CurlHandle’ has no effect in [...]/updater/index.php on line 30

Maybe \CurlHandle on the type hint for getCurl() too?

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