Performance improvement #8
Conversation
grammit
force-pushed
the
improvement/performance_improvement
branch
from
7610b1c
to
5f855f2
Compare
|
Thanks, @grammit, for your work, that looks like it took quite some effort! Since this will most likely be a major release with lots of improvements and new features (e.g. the normalizer), can you please include a |
limenet
force-pushed
the
improvement/performance_improvement
branch
from
5a6d415
to
a98ccf0
Compare
.github/workflows/php.yml
Outdated
| @@ -62,6 +62,10 @@ jobs: | |||
| - name: Install composer dependencies | |||
| uses: ramsey/composer-install@v2 | |||
|
|
|||
| - if: matrix.php-versions == '8.2' | |||
| name: Set phpstan configuration for PHP 8.2 | |||
| run: cat phpstan-php8.2.neon > phpstan.neon | |||
There was a problem hiding this comment.
Based on the contents of phpstan-php8.2.neon this could also be a mv instead of a cat (since you're using > and not >>). mv would probably make the intention a bit clearer.
Also, and this might be more important, Pimcore 10 does not allow PHP 8.2 so I don't think we even need to consider PHP 8.2 as long as we don't offer compatibility with Pimcore 11.
| @@ -4,7 +4,7 @@ | |||
|
|
|||
| trait ClassBasenameTrait | |||
| { | |||
| protected function classBasename(string|object $class): string | |||
| protected static function classBasename(string|object $class): string | |||
There was a problem hiding this comment.
Considering the trait now only contains a single static method, we could also refactor it to be a class. What do you think?
| $loader->load('services.yml'); | ||
| $loader->load('serializer.yaml'); |
There was a problem hiding this comment.
One file is .yml, one .yaml. Should we change services to end in yaml, too?
src/Resources/config/serializer.yaml
Outdated
| valantic.dataquality.serializer.normalizer.datetime: | ||
| class: Symfony\Component\Serializer\Normalizer\DateTimeNormalizer | ||
| tags: | ||
| - { name: valantic.dataquality.serializer.normalizer, priority: -910 } |
There was a problem hiding this comment.
Can you please add a comment explaining the value of `-910?
|
|
||
| Valantic\DataQualityBundle\EventListener\AdminObjectListListener: | ||
| # Pimcore Grid column configurations |
There was a problem hiding this comment.
This probably replaces Valantic\DataQualityBundle\Model\DataType\Score, correct? If so, can you please remove that file and also src/Resources/config/pimcore/config.yml?
| width: 300, | ||
| height: 470, | ||
| width: 380, | ||
| height: 600, |
There was a problem hiding this comment.
| height: 600, | |
| height: 620, |
Currently the last row of text is slightly cut off
| } | ||
|
|
||
| $scores = $this->validation->scores(); | ||
| $score = $scores[$this->context['language']] ?? (array_sum($scores) / count($scores)); |
There was a problem hiding this comment.
Division by zero if no scores are configured.
| @@ -132,9 +126,6 @@ function(ItemInterface $item) use ($obj, $userConfig, $cacheService, $request, $ | |||
| public function checkAction( | |||
There was a problem hiding this comment.
When using the Pimcore Demo setup, there's a custom class in use for Car (App\Model\Product\Car). Unfortunately, this does not work since the class Pimcore\Model\DataObject\Car is being configured in the settings. Do you have an idea how we could solve that?
| @@ -64,7 +91,7 @@ public function score(): float | |||
| return 0; | |||
| } | |||
|
|
|||
| $score = $this->calculateScore(); | |||
| $score = array_sum($this->scores()) / count($this->scores()); | |||
…ovement Updates to be Pimcore 10.6 compatible
No description provided.