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

Type the autentication provider passwords as nullable strings #29200

Merged

Conversation

ChristophWurst
Copy link
Member

For historic reasons we couldn't add a nullable type hint before
nullable type hints were supported by our target php versions. This is
now possible.

This change does not break our API, this type was already documented with phpdoc.

Discovered while studying code for #29187

@ChristophWurst ChristophWurst added the 3. to review Waiting for reviews label Oct 13, 2021
@ChristophWurst ChristophWurst added this to the Nextcloud 23 milestone Oct 13, 2021
@ChristophWurst ChristophWurst self-assigned this Oct 13, 2021
@skjnldsv skjnldsv mentioned this pull request Oct 13, 2021
Copy link
Member

@julien-nc julien-nc left a comment

Choose a reason for hiding this comment

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

🚀

@artonge
Copy link
Contributor

artonge commented Oct 13, 2021

Should we clean the phpdoc then ?

@ChristophWurst
Copy link
Member Author

Should we clean the phpdoc then ?

It says @param string|null $password, doesn't it?

@artonge
Copy link
Contributor

artonge commented Oct 13, 2021

Should we clean the phpdoc then ?

It says @param string|null $password, doesn't it?

Yes, but should we remove them ?

For historic reasons we couldn't add a nullable type hint before
nullable type hints were supported by our target php versions. This is
now possible.

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@ChristophWurst ChristophWurst force-pushed the enhancement/auth-token-provider-create-passwort-type-hint branch from 761f4cc to 01b8291 Compare October 13, 2021 15:17
@ChristophWurst
Copy link
Member Author

I dropped the duplicated phpdoc from the implementation. On the interface it should stay IMO.

@kesselb kesselb added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Oct 13, 2021
@kesselb kesselb merged commit efc3ed5 into master Oct 13, 2021
@kesselb kesselb deleted the enhancement/auth-token-provider-create-passwort-type-hint branch October 13, 2021 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants