Skip to content

Conversation

@nickvergessen
Copy link
Member

Summary

Checklist

@nickvergessen nickvergessen added 3. to review Waiting for reviews technical debt feature: profile PRs or issues related to the Profile feature (e.g. Profile page, API, etc.) labels Oct 23, 2023
@nickvergessen nickvergessen added this to the Nextcloud 28 milestone Oct 23, 2023
@nickvergessen nickvergessen self-assigned this Oct 23, 2023
@nickvergessen nickvergessen force-pushed the feat/noid/profile-manager-ocp branch from e353eee to fd9f7a7 Compare October 23, 2023 10:56
use function Safe\usort;
use OCP\Profile\IProfileManager;
use function array_flip;
use function usort;
Copy link
Member

Choose a reason for hiding this comment

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

Would you please educate me on the reason for this change?

Copy link
Member Author

@nickvergessen nickvergessen Oct 23, 2023

Choose a reason for hiding this comment

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

It was never intended to use functions of an indirect dependency of server 3rdparty (web-auth/webauthn-lib)
The problem is if you import the function using PHPStorm, it will always import them from the library, ignoring the fact it's an indirect dependency of the project, so any update of the library using it would drop it and break this usage by accident.

Copy link
Member

Choose a reason for hiding this comment

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

Got it. Thank you for the explanation.

namespace OC\Core\Db;

use function Safe\json_decode;
use function Safe\json_encode;
Copy link
Member

Choose a reason for hiding this comment

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

I want to know whether there is a standard in place for using safe functions in the codebase.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will send a follow up adjusting all of them

@nickvergessen nickvergessen merged commit 834c9a2 into master Oct 23, 2023
@nickvergessen nickvergessen deleted the feat/noid/profile-manager-ocp branch October 23, 2023 13:15
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 feature: profile PRs or issues related to the Profile feature (e.g. Profile page, API, etc.) technical debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants