Skip to content

Conversation

@fesor
Copy link

@fesor fesor commented Jul 28, 2018

Q A
Type feature
BC Break no
Fixed issues Partially #1798

Summary

This PR adds ability to define views using schema manager. This is small rework of #2510

$schema->createView('my_view', 'SELECT * FROM some_table');
if ($schema->hasView('my_view')) {
    $view = $schema->getView('my_view');
    $schema->dropView('my_view');    
}
  • Base implementation
  • Update docs

@fesor fesor changed the title Schemas with views WIP: Schemas with views Jul 28, 2018
@fesor fesor force-pushed the schema-views branch 6 times, most recently from e33d50c to 7256059 Compare July 28, 2018 23:47
@morozov morozov self-assigned this Aug 28, 2018
@fesor fesor force-pushed the schema-views branch 5 times, most recently from ba974fe to acb6f9f Compare November 10, 2018 22:35
@fesor fesor changed the title WIP: Schemas with views Schemas with views Nov 10, 2018
Comparation schemas with views

Created ViewVisitor interface

Removed unused namespaces
@fesor
Copy link
Author

fesor commented Nov 11, 2018

@morozov could you review this PR when you will have a little time? At least to set some kind of direction of what should I do next.

Copy link
Member

@ostrolucky ostrolucky left a comment

Choose a reason for hiding this comment

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

You should use parameter and return types everywhere

@ostrolucky
Copy link
Member

There are still missing typehints, please go through your PR and add it everywhere. I've seen missing return bool, return void, return self etc. I know ideally CI should tell you to do that, but this rule is not enabled due to old code.

@fesor
Copy link
Author

fesor commented Nov 11, 2018

@ostrolucky as far as I can see there is no any return type hints used in code base yet. I will add type hints/return type to methods introduced by me (so there would be no or little conflicts), but I would suggest to add type hints to other methods as part of separate PR.

Also, I could try to use psalm/psalter to automatically update typehints in code base from inferred types

updated: Looks like @morozov already planning to do something like that: vimeo/psalm#1073

@morozov
Copy link
Member

morozov commented Nov 12, 2018

@ostrolucky as far as I can see there is no any return type hints used in code base yet. I will add type hints/return type to methods introduced by me (so there would be no or little conflicts), but I would suggest to add type hints to other methods as part of separate PR.

Also, I could try to use psalm/psalter to automatically update typehints in code base from inferred types

updated: Looks like @morozov already planning to do something like that: vimeo/psalm#1073

Please use all possible type hints for new APIs. No need to update existing code. I'm working on these changes against develop since it's a BC break.

@morozov
Copy link
Member

morozov commented Nov 12, 2018

@fesor I haven't had a chance to dive deep in the code but on the surface, it all looks good. Please continue to the extent where you consider it code complete and we can get back to it. Or do you have any specific questions?

Base automatically changed from master to 4.0.x January 22, 2021 07:44
@morozov morozov closed this Oct 26, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants