synolia / SyliusGDPRPlugin

Make your Sylius project GDPR compliant.
European Union Public License 1.2
18 stars 8 forks source link

Make default mappings optional #24

Closed chadyred closed 3 years ago

chadyred commented 3 years ago
Q A
Bug fix? no
New feature? yes
BC breaks? no

Hello,

Firstly, thanks for your work. When I use this plugin I notice that it could be great when we override paths for mappings, to keep the default only if we don't define others in configure, in order to not mix what we expected to do in our own mapping VS the default one.

In fact, this is the first time I use it, maybe I misunderstand something :)

CLAassistant commented 3 years ago

CLA assistant check
All committers have signed the CLA.

chadyred commented 3 years ago

Your welcome ^^

chadyred commented 3 years ago

@oallain It is good for the typo ^^

GalloisLuca commented 3 years ago

Hello @chadyred, thanks for the PR ! I'm not sure if i understand well what you did. So, what I understand is that if someone make a new configuration path to override or to add configuration for an entity, every default configuration will be erased ? If that's so, I'm not sure that's a good thing to include in the plugin, not like this. We could make an option to decide if the user wants to keep the default configuration or not, but that's not the same dev.

chadyred commented 3 years ago

I understand, something like a trigger to "enabled" the default mappings or nor, sure ! I check that soon, have a nice week-end

GalloisLuca commented 3 years ago

@chadyred Sounds good to me. If you can sign the license/cla with your account Florian Cellier that would be great :slightly_smiling_face:

chadyred commented 3 years ago

@GalloisLuca Thanks ! I tried to add the other email to my account but the licence/cla seems not to be validatedt with my other email. But no problem, I rebase and I have author with the first email address

GalloisLuca commented 3 years ago

@chadyred No probleme ! Thanks again for the contribution :smile:

@oallain Can you merge it plz ?

oallain commented 3 years ago

Thanks @chadyred :rocket: