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

Adds possibility to set user documentation l10n. #17870

Closed
wants to merge 2 commits into from

Conversation

pierreozoux
Copy link
Member

I still need to figure out how is the documentation shipped in the Nextcloud package, so I can make sure that pt, fr and de are also shipped.

Relates to nextcloud/documentation#216

Thanks for your help!

Signed-off-by: pierreozoux <pierre@ozoux.net>
@kesselb
Copy link
Contributor

kesselb commented Nov 8, 2019

Hmm. nextcloud-17.0.0.zip is already ~80mb big. Around 25mb are for the english documentation. Could you imagine to offer the documentation for other languages as apps via the app store? We probably need some way how to register the app (e.g. user_manual_de) to the help controller. Probably shipping also english as app might reduce the archive and makes it easier to ship doc updates.

@pierreozoux
Copy link
Member Author

@kesselb I just checked, removing images and zipping the user_manual folder, it weights only 35105bits. So adding one language documentation, is just 35kb, adding the ~100language would add 3.5mb. I agree that size does matter, and it is important for various reasons, but I propose that we discuss about it in another issue.

@@ -67,15 +76,35 @@ public function __construct(
* @NoAdminRequired
*/
public function help(string $mode = 'user'): TemplateResponse {
$this->navigationManager->setActiveEntry('help');
define('TRANSLATED_LANGUAGES', array(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make this a class member. e.g: private const TRANSLATED_LANGUAGES

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also prefer a better name like DOCUMENTATION_LANGUAGES

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix when in front of computer, thanks for the advice.

@kesselb
Copy link
Contributor

kesselb commented Nov 9, 2019

Just had a look at user_manual_de and it looks outdated to me. We should not ship outdated documentation because this will lead to confusion. Have you thought about the idea of shipping docs for other languages as apps?

@pierreozoux
Copy link
Member Author

Agree, I'll remove the DE, and check the brazilian.
We still have to do the french. Anyway, we have to coordinateur between the documentation repot and here.

About the app, Im maybe biased because I don't want to create a repo, create an app, write tests, write a CICD pipeline and maintain an app for this 5 lines PR, but I think Nextcliud has to be consistent.
I'm not opposed to the idea of splitting into apps, but IMHO, the split has to happen, either at the i18n or the documentation.

Currently Nextcloud ships i18n for the app itself and the documentation. So to be consistent, I think it has to ship the documentation translated too.
But I wouldn't be mad if Nextcloud decides that i18n or documentation (or both even) is now an optional package.

But again, I think this is another discussion than this PR.

Copy link
Member

@juliushaertl juliushaertl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm strongly against doing this until there is a proper way to have translated documentation somehow kept up to date with the English one. Current de and pt_BR didn't receive an update in the last half year.

Besides that, maybe we should find a way to not ship the duplicate documentations (with images) as individual folders but somehow just have the text translations?

@pierreozoux
Copy link
Member Author

Well :) the english documentation is not that up to date either ;)

https://github.com/nextcloud/documentation/blob/master/user_manual/whats_new.rst (check the date of the last change)

Im also tackling this translation thing here

nextcloud/documentation#216 (comment)

I'm not asking to merge right away, I'm asking for a bit of support to help me roll the ball around this topic:) thanks for your help :)

The next thing I need is to understand, where is the CI that packages the Nextcloud zip and fetch documentations so I can make sure it fetches the right thing at the right time.

@ChristophWurst
Copy link
Member

I'd say let's close this for now. This hasn't received any update in quite a while and this is something that needs to be discussed more.

@pierreozoux
Copy link
Member Author

@ChristophWurst I'm still waiting nextcloud/documentation#1824 to be merged first.

Can we keep it open? (and help move the other one?)

@ChristophWurst ChristophWurst reopened this Jul 8, 2020
@pierreozoux
Copy link
Member Author

So :)

image

Now we have an automated way to translate documentation \o/

But it is still not shipped inside Nextcloud:

image

(this screenshot is 18.0.14)

I have a simple question, where (I searched) is the line that adds the documentation inside the Nextcloud zip? I just need the answer to that, and then I'll do the necessary PR that is needed to be solved before this PR :)

@skjnldsv skjnldsv added 2. developing Work in progress and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Feb 21, 2024
@skjnldsv
Copy link
Member

Hey @pierreozoux seems like this PR is quite old and the direction since changed.
As there is no feedback since a while I will close this ticket.

Thanks for the interest in Nextcloud and the effort put into this! 🙇

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

Successfully merging this pull request may close these issues.

None yet

5 participants