-
-
Notifications
You must be signed in to change notification settings - Fork 21
feat: unique aliases per locale #291
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
🦋 Changeset detectedLatest commit: 1892da5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #291 +/- ##
==========================================
- Coverage 43.10% 40.98% -2.12%
==========================================
Files 2 2
Lines 638 627 -11
Branches 157 153 -4
==========================================
- Hits 275 257 -18
- Misses 291 296 +5
- Partials 72 74 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
boazpoolman
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.
Hi @jorrit,
Thanks for this PR. This looks like a well documented and tested feature that will be a great addition to the plugin!
I've review your work and at first glance it looks to be perfectly in order. One thing I did notice is there is one test case that isn't working as expected, which is the following:
This PR, without putting unique_per_locale on true, allows duplicate URLs across different translations of the same document. This is unexpected behavior. In this case it should suffix the URLs with -0. I've written a test case for this specific situation which should be failing now. Could you address that?
Other than that it seems to be working great!
b96c8ba to
97668a8
Compare
boazpoolman
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.
LGTM!
What does it do?
Adds an option to generate unique urls per locale instead of across the entire site.
Why is it needed?
When the locale is already part of the URL (for instance, in the domain name or an externally provided path prefix) the URL does not need to be unique for the site, but just for the locale.
How to test it?
unique_per_localeto true.Related issue(s)/PR(s)
Addresses #289