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

Run make gettext and add new/modified files #458

Closed
wants to merge 1 commit into from
Closed

Conversation

ywwg
Copy link
Member

@ywwg ywwg commented Nov 29, 2021

No description provided.

@ywwg ywwg changed the base branch from 2.3 to main November 29, 2021 17:18
@ywwg ywwg mentioned this pull request Nov 29, 2021
@ywwg
Copy link
Member Author

ywwg commented Nov 30, 2021

There are a whole bunch of broken links in the manual... should I fix / remove those here or can we address it later? (I'd prefer to do this incrementally)

@Swiftb0y
Copy link
Member

Most of those seem to be false positives. Either because some conncections actually seem to block us (403) or because there is some subtle TLS error. All of the links do actually work in the browser.

@daschuer
Copy link
Member

Mm, can we add them to a kind of ignore list?
Otherwise it defeats the benefit of the entire check.

@ywwg
Copy link
Member Author

ywwg commented Dec 1, 2021

I have never worked with this check so I will have to find some time to figure out what options it takes

@ywwg
Copy link
Member Author

ywwg commented Dec 1, 2021

(alternatively, we can ignore it for this PR and file a bug)

@Swiftb0y
Copy link
Member

Swiftb0y commented Dec 1, 2021

More importantly though, who can review this PR? I've never ran this command so I assume it just updates the translation files? So we can basically merge without looking at them, right?

@Holzhaus
Copy link
Member

Holzhaus commented Dec 1, 2021

IIRC @esbrandt is the translations expert.

@daschuer
Copy link
Member

daschuer commented Dec 2, 2021

Here is the guide for our mixxx repro.
https://github.com/mixxxdj/mixxx/wiki/internationalization

I think the same workflow applies here as well.
We normally push the changes directly to transiflex and into our repro.
No need for a PR.

I can do this here as well. And we can close this PR.
Shall I?

@ywwg
Copy link
Member Author

ywwg commented Dec 2, 2021

I had to run this command to get the manual to pick up my new controller file, and it made all these other updates too

@ywwg
Copy link
Member Author

ywwg commented Dec 2, 2021

Is there something else I should have done in my other PR to get the build system to see my new file?

quote:

Update source translations

For every change to the manual source files (.rst) the source translation files
(.pot) must be re-generated. These are stored in source/locale/pot and contain
the text of every English phrase in the manual in a common format used for
translation and can be regenerated with:

make gettext

@daschuer
Copy link
Member

daschuer commented Dec 2, 2021

Ah, yes I see the steps are described here:
https://github.com/mixxxdj/manual#readme

I think you should do everything step by step.
This PR is only the first step.

I think you can also push directly, too keep transiflex and the manual repository in sync.

Do you have sufficient rights at transiflex?

Is there something else I should have done in my other PR to get the build system to see my new file?

You can already see your new file in the deploy preview:
https://deploy-preview-456--mixxx-manual.netlify.app/hardware/controllers/yaeltex_minimixxx.html
I think this PR must be merged first, before you can add it here:
https://github.com/mixxxdj/manual/blob/2.3/.tx/config

@Holzhaus
Copy link
Member

Holzhaus commented Dec 2, 2021

I had to run this command to get the manual to pick up my new controller file, and it made all these other updates too

This is not necessary. With a clean build dir, the new controller documentation is included.

It probably makes sense to update the translations because they are old, but it's not necessary for building the manual.

@ywwg
Copy link
Member Author

ywwg commented Dec 3, 2021

ok, closing this

@ywwg ywwg closed this Dec 3, 2021
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

Successfully merging this pull request may close these issues.

None yet

4 participants