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

Added ActiveRecord extension type to support findBySql method usages #6

Merged
merged 4 commits into from
May 28, 2024

Conversation

iVovolk
Copy link
Contributor

@iVovolk iVovolk commented May 13, 2024

I'm just in a couple of days long relationship with phpstan, trying to integrate it into a legacy project i'm currently working on.

While switching options i found out that your extension suits it the best. Though i found some cases where it lacks some features.

In this PR i'm proposing an update to add support for ActiveRecord::findBySql() which i use in my project. And even though i have not tested it i belive that this update also allows easy implemetation of ActiveRecord::findByCondition() support.

@iVovolk
Copy link
Contributor Author

iVovolk commented May 16, 2024

@erickskrauch hello, could you please take a look at this?

@erickskrauch
Copy link
Owner

@erickskrauch hello, could you please take a look at this?

Hello. Yes, I have already seen this PR. Unfortunately, it requires time to read and understand what you have submitted. And at the moment I don't have it :( I'll take a look at this PR later.

Until then you can just use your fork. Take a look at this article to learn how to.

@iVovolk
Copy link
Contributor Author

iVovolk commented May 16, 2024 via email

Copy link
Owner

@erickskrauch erickskrauch left a comment

Choose a reason for hiding this comment

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

I apologize for the long wait. I have reviewed your PR. Well done! Please review the comments I posted and we can merge this.

.gitignore Outdated Show resolved Hide resolved
tests/Yii/Comment.php Outdated Show resolved Hide resolved
@erickskrauch erickskrauch added the enhancement New feature or request label May 23, 2024
@erickskrauch erickskrauch merged commit f841ad3 into erickskrauch:master May 28, 2024
4 checks passed
@erickskrauch
Copy link
Owner

@iVovolk, thank you very much for this PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants