Adding columns defined in actAs-templates to the docblock of the generated model class#110
Conversation
It is an interesting idea, to provide better developer experience. However, I am not sure about the time to invest on this new feature. In any case, I like to read tests as they show examples. |
|
I am a progressive person. If the change will move your work forward or you see it as helping others in their work then go for it. |
69fefea to
a2ed8ed
Compare
|
I've updated the pr:
wdyt? |
|
@thirsch it is better than before. Let's add this new behaviour and make it with a good job. How do you feel when making changes in this class? |
a2ed8ed to
b6356a9
Compare
Well, I still think we should keep changes in the sense of new features to a bare minimum or try to avoid it at all. For this particular topic, it was more like a bug to us, as this saves a lot of manual added typehints in the codebase. Sure, it is no bug as such, as it was absolutely not a topic back in the days when this code base was written. |
06817da to
e8aac05
Compare
What does forces you to add these type hints ? I have 2 hypotheses of usage and constraints, but I wish to know yours. |
e8aac05 to
7d67309
Compare
7d67309 to
95da927
Compare
It's primarly a question of inconsistency of having the "direct" fields documented whilst the "inherited" once are not. Points are: ide support (code completion, type checks), tools like psalm/phpstan and even good old pull request review, where the fields are there and can be found by checking what has been created/changed. |
95da927 to
3de830a
Compare
3de830a to
cb28ccb
Compare
cb28ccb to
9b6de4a
Compare
b233339 to
3e49b73
Compare
|
I've changed the loosely testing "contains"-checks to a snapshot testing to avoid sideeffects being generated. IMHO this even allows a clearer view on what exactly is this change doing. |
7908c77 to
f6bf135
Compare
fca34fd to
b78fdd3
Compare
505b409 to
a3e2a22
Compare
thePanz
left a comment
There was a problem hiding this comment.
Some minor comments, LGTM otherwise!
Thank you for your work and commitment! Kudos!
…rated model class.
a3e2a22 to
4bf5a41
Compare
|
Thanks for merging it :) |
The generated docblock does not contain the columns defined in the referenced behaviours (actAs). This patch adds those columns to the docblock.
@alquerci, @connorhu, @thePanz, wdyt about this addition? I'd create a test for it, if you are interested in the change at all.