-
Notifications
You must be signed in to change notification settings - Fork 19
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
fix: Move from private OC\HintException
to public OCP\HintException
#514
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This breaks all the consumers of the API, because OCP does not extend the former exception, so any catch does not work anymore?🤔
OC\HintException
to public OCP\HintException
OC\HintException
to public OCP\HintException
No basically we can use OCP because users of the API that still throw the OC exception will still work -> OC extends OCP meaning as we catch OCP we also catch OC.
|
OC\HintException
to public OCP\HintException
OC\HintException
to public OCP\HintException
This part is exactly the right one. You don't find any consumers of the API because you look for the wrong one. https://github.com/search?q=org%3Anextcloud%20ValidatePasswordPolicyEvent&type=code Used in our org in 4 repos at least:
So the correct way would be to document it in the dev docs that OCP should be catched now and then we can switch to it in some releases |
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! |
As @nickvergessen mentioned we can not replace this directly but need to deprecate first. |
Signed-off-by: Ferdinand Thiessen <[email protected]>
54f8bb1
to
f99cfdf
Compare
I checked all using apps in our organization, all migrated to So I think we should be good for the Nextcloud 31 cycle. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approve the idea, did not look into the details of whether it’s breaking, trusting you no this
We should not use private API in apps, especially if there is a public alternative.