-
Notifications
You must be signed in to change notification settings - Fork 213
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
Duplicate missing content from default language during build #612
Conversation
Signed-off-by: lianmakesthings <[email protected]>
Signed-off-by: lianmakesthings <[email protected]>
Signed-off-by: lianmakesthings <[email protected]>
Signed-off-by: lianmakesthings <[email protected]>
Signed-off-by: lianmakesthings <[email protected]>
Signed-off-by: lianmakesthings <[email protected]>
Signed-off-by: lianmakesthings <[email protected]>
Signed-off-by: lianmakesthings <[email protected]>
✅ Deploy Preview for tag-app-delivery ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Action RequiredYou are adding or updating English content so please take the following actions for other languages.
|
Signed-off-by: lianmakesthings <[email protected]>
@cjyabraham could you also take a look at this please? 🙏 |
Signed-off-by: lianmakesthings <[email protected]>
This is a nice solution. One concern, it will create multiple copies of the same content. Should we define the canonical url of copies so that we don't get flagged by Google for duplicate content or the wrong one gets prioritized in the search rankings? Is this a legitimate risk we should address? |
Signed-off-by: lianmakesthings <[email protected]>
ad1459a
to
00dbdb4
Compare
This is for SEO purposes? And we already have this situation right now with the manually duplicated content, right? I figure the easiest way to do that would be to add the canonical url to the front matter so it can be easily duplicated. I can look into that and if it's not too much work add this. On the other hand, our website is fairly small and I'm not sure how big of a problem this actually poses. |
This reverts commit 78904bb. Signed-off-by: lianmakesthings <[email protected]>
Signed-off-by: lianmakesthings <[email protected]>
00dbdb4
to
807b6b6
Compare
Yes, this is for SEO. Yes, we already have that situation. True, I'm not sure this is a real problem but just thought I'd raise it in case anyone else knew. |
Thinking a bit more about the UX of this solution, is it weird that a person who is browsing the site in French, for example, would suddenly come across a page in English that should be in French? How would they interpret that? Would they possibly conclude the site is broken for seeing an English page on a To improve this usability, it may help to add a message at the top of such a page saying something like this (but in French): "This page has not yet been translated into French, however, here is the English version of the page for now." Any thoughts? |
@cjyabraham I will also open three additional issues to evaluate if we want to add these features: |
Signed-off-by: lianmakesthings <[email protected]>
@lianmakesthings |
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.
This will be an awesome addition and I can so see how it will continue to improve! Well done on tackling the big first push @lianmakesthings !
I made a few nit comments, but have two bigger questions...
- When I navigate the site locally I see languages appear and disappear per page based on if there is a translation. For example, on the Operator White paper I see Chinese, Korean and Japanese. But on the platforms maturity model I don't see Korean. And neither page has the Spanish and French additions that have come recently.
- With this new model do we have any need for
.gitignore
to manage things? I can't quite reason about it right now, since we definitely need people to be able to commit to these directories, but also a bunch of the files are sorta ignored now... 🤔
- If you update content, that has corresponding files in other languages, include a note suggesting that users check the English page for the most recent updates in those translated pages. | ||
- If you add new content under `website/content/en` there is nothing you need to do. |
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.
👏 This will be a much better experience! I have added a comment to #615 to possibly even improve on this even more 💪
Co-authored-by: abangser <[email protected]> Signed-off-by: Lian Li <[email protected]>
Thanks fro the review @abangser 🙏 To address the comments:
Yes, the behaviour will be different for dev and staging (deploy preview) / prod, due to the flow. On dev, the content is served directly from
I figured for dev purposes it's okay/better to have the duplicated stuff out of the way and if you really need to get a 1:1 mirror, you can also manually execute the script and then run
I don't think |
Ahhhh I read that comment but hadn't understood it. Thanks for the clarification. So long as it is working as expected in prod I think that is fine 👍
As I said, it was me thinking out loud. I guess what I am concerned with is someone has all these files locally and commits them thereby overwriting the dynamic creation introduced in this PR (and possibly causing drift etc). But realistically we will spot that in a PR review so likely not a concern. Sorry for the noise, but thanks for the space to think it through! I will add approval here, but I realise it is a bit ceremonial as I can not merge. Might help with other people's reviews though! |
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.
I am not a binding review, but I have read through this, asked some questions and feel good about this change. I am really excited to lower the cost to add and manage translated content to the site!
Yeah, the issue is that we cannot know which files were added due to the script and which ones are genuinely translated files, unless a human being looks at them. And in a normal dev workflow you shouldn't need to duplicate the files locally anyway. So my suggestion would be to catch these in PR reviews. Thanks for voicing and talking through your concerns! |
Signed-off-by: lianmakesthings <[email protected]>
Signed-off-by: lianmakesthings <[email protected]>
Signed-off-by: lianmakesthings <[email protected]>
Signed-off-by: lianmakesthings <[email protected]>
Signed-off-by: lianmakesthings <[email protected]>
Signed-off-by: Chris Abraham <[email protected]>
Signed-off-by: lianmakesthings <[email protected]>
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.
This is awesome - thanks @lianmakesthings! Lgtm
Closes #611
Introducing a bunch of changes in this PR to make dealing with languages much easier.
The Problem
Hugo expects the entire content for a language to exist in a single folder, e.g.
content/zh
. Currently, we are asking everyone who introduces a new language or new content to duplicate all english content to all languages.The Solution
Instead of manually duplicating content I have introduced a script which iterates over all files in the default language folder (
en
) and duplicates all files that do not exist in the target language, By using the-L
flag this command also handles symlinks, i.e. it duplicates the linked content.This script will run before the preview and production builds.
What Changes
duplicate_missing_content
script is explicitly triggered