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

Draft:Add interface and add typehinting #4130

Merged

Conversation

ColonelMoutarde
Copy link
Contributor

Add the DatabaseDAOInterface

@ColonelMoutarde ColonelMoutarde changed the title Add interface and add typehinting Draft:Add interface and add typehinting Jan 7, 2022
@@ -3,7 +3,7 @@
/**
* This class is used to test database is well-constructed.
*/
class FreshRSS_DatabaseDAO extends Minz_ModelPdo {
class FreshRSS_DatabaseDAO extends Minz_ModelPdo implements DatabaseDAOInterface {
Copy link
Member

Choose a reason for hiding this comment

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

Why an interface when there is already a base class FreshRSS_DatabaseDAO?

Comment on lines 16 to 19
/**
* @param array $dao
* @return array
*/
Copy link
Member

Choose a reason for hiding this comment

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

Comments can be removed when there is no additional information than the native type hints

@Alkarex Alkarex added the System care Everything related to the system care label Jan 7, 2022
@Alkarex Alkarex added this to the 1.19.2 milestone Jan 7, 2022
@math-GH
Copy link
Contributor

math-GH commented Jan 17, 2022

Is it a draft or an open PR?

@ColonelMoutarde
Copy link
Contributor Author

A draft

@math-GH math-GH marked this pull request as draft January 18, 2022 07:06
@Alkarex Alkarex modified the milestones: 1.19.2, 1.20.0 Jan 24, 2022
@Alkarex Alkarex marked this pull request as ready for review February 5, 2022 10:37
@Alkarex
Copy link
Member

Alkarex commented Feb 5, 2022

I have simplified and finalised this PR.
For inspiration, more type hints welcome from the list of those compatible with PHP 7.0 https://php.net/language.types.declarations
In particular: bool, float, int, string
As contribution to #4112

@Alkarex Alkarex merged commit 87b181a into FreshRSS:edge Feb 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
System care Everything related to the system care
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants