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

chore: update php intl stub and fix type issues #50452

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Jan 27, 2025

Summary

  • Update the stub for PHP 8.0+
  • Adjust using code to comply

Checklist

@susnux susnux added 3. to review Waiting for reviews technical debt php Pull requests that update Php code ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring) labels Jan 27, 2025
@susnux susnux added this to the Nextcloud 32 milestone Jan 27, 2025
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux requested review from come-nc, provokateurin and nickvergessen and removed request for come-nc February 11, 2025 11:06
@susnux susnux marked this pull request as ready for review February 11, 2025 11:06
$qb = $this->connection->getQueryBuilder();
return $qb
->select('storage_id', 'user_id')
->from('mounts', 'm')
->innerJoin('m', 'filecache', 'f', $qb->expr()->eq('m.storage_id', 'f.storage'))
->where($qb->expr()->eq('mount_id', $qb->createNamedParameter($mountId, IQueryBuilder::PARAM_INT)))
->andWhere($qb->expr()->eq('path_hash', $qb->createNamedParameter($pathHash, IQueryBuilder::PARAM_STR)))
->execute()
->executeQuery()
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought that changes the exception thrown, but it’s actually not clear.
Looking into that made me spot https://github.com/nextcloud/server/pull/46547/files#r1950850746

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Where is the exception changed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Well I dunno, it was a memory that the exception class is not always the same.

executeQuery wraps \Doctrine\DBAL\Exception in OC\DB\Exceptions\DbalException, execute does not.

So for intance I think the try/catch here: https://github.com/nextcloud/server/pull/50452/files/70b9ce8d579c55e22885c99d0c45cfdc080bb903#diff-e6c27e6e12c5df9554aee0a8c9fa6c3c8aca1e33821c4a32ebb8fc1fd1f40197R116
Might need to be adapted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well than revert that part for now as it is unrelated?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews php Pull requests that update Php code ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring) technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants