Skip to content
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

As contributor, I can add a group of domains (and not only domains) in a group of domains #441

Open
MaartenLMEM opened this issue Sep 2, 2021 · 4 comments

Comments

@MaartenLMEM
Copy link
Contributor

MaartenLMEM commented Sep 2, 2021

Example : "Habillement et prêt à porter" will contain group of domains "Habillement luxe homme" et "habillement luxe mixte + other domains

Please first size it and we ll see

@lutangar
Copy link
Member

lutangar commented Sep 6, 2021

I'd say Size S 👕 as long as we stick to one level of recursion... but there might not be validation everywhere to ensure that's actually the case. We'll only fetch domainSets inside a domainSet and that's it.

This will also generate more matchintContext so might be a performance impact.

This is handle backend side so I'd say there are no impact on any frontend side.

@MaartenLMEM
Copy link
Contributor Author

From my contributor point of view, this 1st level would be useful.
@lutangar Do you see any risk of breaking something, or are you completely confident in and outs of this feature ?

but there might not be validation everywhere to ensure that's actually the case.

Idea : To be sure there is no mistake we will write "2nd" (for 2nd level) in each title of a group of domain that contains other groups.

@lutangar
Copy link
Member

lutangar commented Sep 6, 2021

Just a warning note on the MatchingContext v4 normalizer wich probably need fixing already...
V4 should use $matchingContext->getFullUrlRegex($this->escaper) as in v3

@MaartenLMEM
Copy link
Contributor Author

Postponed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants