Bake task for filter collections #300
Conversation
Codecov Report
@@ Coverage Diff @@
## master #300 +/- ##
============================================
- Coverage 93.92% 93.38% -0.54%
- Complexity 204 227 +23
============================================
Files 17 18 +1
Lines 510 590 +80
============================================
+ Hits 479 551 +72
- Misses 31 39 +8
Continue to review full report at Codecov.
|
|
This whole file system generation and comparison thing is a bit strange |
That's the problem with using |
|
Any takes on what needs to be bumped? Or should we completely rewrite file based generation testing? |
|
My guess would be it's the twig view plugin for which a particular big bugfix is required. |
|
Otherwise all good? Or do you see any concerns so far? |
|
Tests are now fixed :) |
| */ | ||
| public function initialize(): void | ||
| { | ||
| $this->value('author_id'); |
There was a problem hiding this comment.
Would be nice if instead of repeating $this on each line method chaining was used similar to https://github.com/cakephp/bake/blob/85be666e8fbcc4a67ff0c0c7a66221623793cd5a/tests/comparisons/Model/testBakeTableValidation.php#L51-L53
There was a problem hiding this comment.
Yeah, for simplicity of the command, I decided not to do this.
Do you have an idea on how to easily achieve that?
For me, it seems enough as is, since it works both equally, and the lines don't increase.
So I am fine either way.
There was a problem hiding this comment.
Yes it's the same number of lines but avoid repeatation of $this would be nice.
You can check bake's temple for tables and see how it generates the chaining. It should be just a matter of tweaking a loop but if it's too much trouble we can keep as it is.
|
CS job is still failing, wonder why stickler hasn't fixed it. |
dereuromark
force-pushed
the
bake-task
branch
from
d00708c to
3136c16
Compare
|
The CS fails seemed unrelated, but I fixed them now as well As for the $this chaining. I think this would need a helper method and some additional internal logic. |
| @@ -5,6 +5,9 @@ linters: | |||
| extensions: 'php' | |||
| fixer: true | |||
|
|
|||
| files: | |||
| ignore: ['tests/comparisons/*'] | |||
There was a problem hiding this comment.
@markstory Any idea why this could be ignored in stickler still?
I copied this from https://github.com/cakephp/migrations/blob/master/.stickler.yml#L4-L5
There was a problem hiding this comment.
Are you aiming to have these files not checked? That is what this config would do.
There was a problem hiding this comment.
Unfortunately, FriendsOfCake is using the OAuth version of the integration so we won't see any logs here. If the GitHubApp integration was used we'd get better logs and annotations instead of comments.
There was a problem hiding this comment.
I have done Stickler's GitHubApp integration now for FOC.
There was a problem hiding this comment.
Stickler respects phpcs.xml right? So it would be better to add the ignores there @dereuromark.
I whipped sth up
Resolves #296
I didn't check the visibility on the entity yet, if I should do that let me know
But this already works quite well for default cases IMO.
We could think about further improvements as follow up: