-
Notifications
You must be signed in to change notification settings - Fork 10
Upgrade sylius 2.0 #65
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
Conversation
5568bc9 to
24c7821
Compare
maxperei
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you very much for your contribution @jbcr and well done 🫰
tests are not passing as you probably notice since you changed annotation to attribute you have to edit tests/PHPUnit/Fixtures/Foo.php to use php attribute
<?php
declare(strict_types=1);
namespace Tests\Synolia\SyliusGDPRPlugin\PHPUnit\Fixtures;
use Synolia\SyliusGDPRPlugin\Attribute\Anonymize;
class Foo
{
#[Anonymize(['faker' => 'email'])]
public $email = '';
#[Anonymize(['value' => 'test-annonation-value'])]
public $value;
#[Anonymize(['value' => 'test-annonation-value-without-property'])]
public $valueWithoutProperty;
#[Anonymize(['faker' => 'email', 'prefix' => 'test-annotation-prefix-'])]
public $prefix;
#[Anonymize(['value' => 'value', 'prefix' => 'test-annotation-prefix-value-'])]
public $prefixValue;
#[Anonymize(['value' => 'attribute'])]
public $mergeYamlAnnotationConfiguration;
#[Anonymize(['value' => ['value1', 'value2']])]
public array $arrayValue;
#[Anonymize(['value' => '1542', 'prefix' => '1542'])]
public int $integer;
public $bar;
}but this may not correct yet because configuration yaml is not merged anymore maybe we'll have to investigate on this..
the remaining is just about code style and some tiny stuff but this a great beginning
Co-authored-by: Maxime Pereira-Lima <7437661+maxperei@users.noreply.github.com>
What is the configuration loader priority? The attribute configuration overrides the YAML configuration. But sometimes, the loader is in a different order and the test works. I propose using the tag priority to set the loader order and fix this error. |
sure @jbcr, |
cf44204 to
bdaa267
Compare
It's changed and all tests pass. |
maxperei
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks wonderful now many thx @jbcr


Upgrade for Sylius 2.0 and Symfony 6.4 and ^7.0
Drop Sylius 1.x support
Drop Symfony 5.x support