Expanded ReadInterface

[dverkade]

• Expanded to ReadInterface to include the readAll function, since a class implementing the ReadInterface should have this function in order to substitute the Read class.

• Removed comment variable declarations from other file where the type is declared. This was necessary due to the missing function in the interface. Autocompleting this function now works properly, so these comments can be removed.

[orlangur]

Interfaces (even non-@api ones) cannot be expanded this way (see http://devdocs.magento.com/guides/v2.1/contributor-guide/backward-compatible-development/index.html#introduction-of-a-method-to-a-class-or-interface).

There are a lot of code yet which rely on implementation rather than on interface, not sure whether readAll was missed here or there was some reasoning behind.

[okorshenko]

I agree with @orlangur about BC policy.

[dverkade]

Hi @okorshenko,

The reason I have created the PR this way is that the ReadFactory class actually has a hard dependency to the Read class. So at this point the Read class does not offer an extension point in Magento. Since the readAll function is called in several other modules I assumed this function has been forgotten to be included in the ReadInterface, so I added this function to that interface. Since no real extension point is offered at this moment I thought I would by wise to change the interface directely. The classes implementing the ReadInterface already have the readAll function in them. Even if someone did manage to overwrite the Read class, the readAll function is still necessary for Magento to even work. So that's why I have changed the interface directly.

If you want this PR to be changed I would be happy to do so. Please guide me how you want me to do with this. I can create a ReadAllInterface which extends the ReadInterface. Then the Read class should implement the ReadAllInterface.

Please share your toughts.

[okorshenko]

Thank you @dverkade for the detailed explanation. In normal case we should follow BC policy. I talked to other Magento Architects in we agreed that in this particular case we can accept this change for 2.3. This interface does not have @api annotation, so we can do this.
Thank you

[dverkade]

Hi @okorshenko,

Thanks. When I make future PR's I'll try to add the reason in the comments why I opted to create the PR in a certain way. Hope this makes it easier for you to accept or reject a certain PR.

[Ctucker9233]

@magento-team Will this be backported to 2.2?