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

FR - CMS chapter #620

Merged
merged 20 commits into from
Jan 9, 2020
Merged

Conversation

JustinyAhin
Copy link
Contributor

@JustinyAhin JustinyAhin commented Jan 8, 2020

Makes progress on #539

I've finally ended up the chapter. It was difficult with many of technical expressions not having an easy or obvious translation in FR. I'd be happy to get a quick review on the PR.

First part of CMS chapter in FR
Add last part of CMS chapter
Update charts titles and descriptions translations.
@JustinyAhin JustinyAhin added the translation world wide web label Jan 8, 2020
Copy link
Member

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

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

Thanks! Yeah can imagine it's tough to do these translations.

A few technical nits with the meta data for you to correct.

Also is it possible for you to run npm run generate from the src directory to generate the HTML version as well? If not familiar with npm then let us know and can do that later.

@borisschapira , @nico3333fr , @allemas , @AymenLoukil , @SilentJMA would be great to have one of you check the translation before we merge if you can.

src/content/fr/2019/cms.md Outdated Show resolved Hide resolved
src/content/fr/2019/cms.md Outdated Show resolved Hide resolved
src/content/fr/2019/cms.md Outdated Show resolved Hide resolved
src/content/fr/2019/cms.md Outdated Show resolved Hide resolved
Copy link
Contributor

@borisschapira borisschapira left a comment

Choose a reason for hiding this comment

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

Excellent work, if you ask. The result is fluid, while using a sustained vocabulary. It's very good work, and given the length of the document, it must have taken a while. Thank you very much for that.

I corrected a few minor typos that needed a little rework, and a small issue regarding markdown links (you need to remove spaces between "]" and "("), but that's nearly nothing.

src/content/fr/2019/cms.md Outdated Show resolved Hide resolved
src/content/fr/2019/cms.md Outdated Show resolved Hide resolved
src/content/fr/2019/cms.md Outdated Show resolved Hide resolved
src/content/fr/2019/cms.md Show resolved Hide resolved
src/content/fr/2019/cms.md Outdated Show resolved Hide resolved
src/content/fr/2019/cms.md Outdated Show resolved Hide resolved
src/content/fr/2019/cms.md Outdated Show resolved Hide resolved
src/content/fr/2019/cms.md Outdated Show resolved Hide resolved
src/content/fr/2019/cms.md Outdated Show resolved Hide resolved
@borisschapira borisschapira self-requested a review January 8, 2020 16:19
Copy link
Contributor

@borisschapira borisschapira left a comment

Choose a reason for hiding this comment

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

I've clicked on "Approve" by error (damn interactive elements that moves while the component's loading). While I approve the translations, there are small fixes to be made.

@JustinyAhin
Copy link
Contributor Author

Thank you @borisschapira for reviewing.
And @bazzadp for the flags you raised. I'll integrate them soon.

JustinyAhin and others added 12 commits January 8, 2020 17:29
Co-Authored-By: Boris Schapira <boris@schapira.dev>
Co-Authored-By: Boris Schapira <boris@schapira.dev>
Co-Authored-By: Boris Schapira <boris@schapira.dev>
Co-Authored-By: Boris Schapira <boris@schapira.dev>
Co-Authored-By: Barry Pollard <barry_pollard@hotmail.com>
Co-Authored-By: Barry Pollard <barry_pollard@hotmail.com>
Co-Authored-By: Barry Pollard <barry_pollard@hotmail.com>
Co-Authored-By: Barry Pollard <barry_pollard@hotmail.com>
Co-Authored-By: Boris Schapira <boris@schapira.dev>
Co-Authored-By: Boris Schapira <boris@schapira.dev>
Co-Authored-By: Boris Schapira <boris@schapira.dev>
@JustinyAhin
Copy link
Contributor Author

JustinyAhin commented Jan 9, 2020

Also is it possible for you to run npm run generate from the src directory to generate the HTML version as well? If not familiar with npm then let us know and can do that later.

Hey @bazzadp. I run npm run generate and it looks to throw an error. Here's the output

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! almanac.httparchive.org@0.0.1 generate: `node ./tools/generate`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the almanac.httparchive.org@0.0.1 generate script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm WARN Local package.json exists, but node_modules missing, did you mean to install?

So I run npm install and npm run generate again but I don't file it generates any new file. Can you look at this please?

Also, I've accepted your others suggestions.

@JustinyAhin
Copy link
Contributor Author

@borisschapira all the changes and typos corrections are integrated :)

Copy link
Contributor

@borisschapira borisschapira left a comment

Choose a reason for hiding this comment

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

LGTM

Capture d’écran 2020-01-09 à 10 34 11

@tunetheweb
Copy link
Member

So I run npm install and npm run generate again but I don't file it generates any new file. Can you look at this please?

Interesting. It would be good to get this sorted for you.

Questions from me:

  • Did npm install run correctly and create stuff in your src/node_modules directory?
  • Any output or error messages from npm install?
  • Any output or error messages from npm run generate or did it run cleanly this time?
  • Did npm run generate look like it was iterating through the chapters?
  • What version of node are you running?

@JustinyAhin
Copy link
Contributor Author

JustinyAhin commented Jan 9, 2020

  • Did npm install run correctly and create stuff in your src/node_modules directory?

Yes, here's npm install output:

added 160 packages from 152 contributors and audited 210 packages in 72.774s
found 2 low severity vulnerabilities
  run `npm audit fix` to fix them, or `npm audit` for details
  • Any output or error messages from npm install?

No

  • Any output or error messages from npm run generate or did it run cleanly this time?

I've got just warnings, not errors

almanac.httparchive.org@0.0.1 generate C:\Users\EubertineAhinon\Desktop\almanac.httparchive.org\src
node ./tools/generate
(node:7300) UnhandledPromiseRejectionWarning: TypeError: object null is not iterable (cannot read property Symbol(Symbol.iterator))
    at generate_chapters (C:\Users\Eubertine Ahinon\Desktop\almanac.httparchive.org\src\tools\generate\generate_chapters.js:27:45)
    at async C:\Users\Eubertine Ahinon\Desktop\almanac.httparchive.org\src\tools\generate\index.js:9:3
(node:7300) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:7300) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.`
  • Did npm run generate look like it was iterating through the chapters?

No, just run at once

  • What version of node are you running?

node 12.14.1 and npm 6.13.4

@tunetheweb
Copy link
Member

Ah this is because us selfish Mac users never set this up to work properly for windows.

If you temporarily change line 26 of src\tools\generate_chapter.js to the following:

    const re = /content\/(.*)\/(.*)\/(.*).md/;

to the following

    const re = /content\\(.*)\\(.*)\\(.*).md/;

Then it will work.

Can you try that and then add the HTML to this PR (but do not commit the change to generate_chapter.js - as don't want to break it for the rest of us).

Raised #622 to see if we can fix this properly.

@mikegeyser
Copy link
Contributor

guiltily puts hand up as selfish mac user

Sorry, will fix it!

@tunetheweb
Copy link
Member

@JustinyAhin , @mikegeyser has now fixed this permenately in 8693c9c and I've merged that in, so if you merge in from upstream master it should work for you now.

Thanks @mikegeyser !

@JustinyAhin
Copy link
Contributor Author

hey @bazzadp just run npm run generate successfully. some files as markup.html were regenarated, and I commit them. you can look at the commits messages.

@tunetheweb tunetheweb merged commit 6d44147 into HTTPArchive:master Jan 9, 2020
@tunetheweb
Copy link
Member

Looks good and merged to master now. Thanks for translating this chaper and hopefully next one will be easier!

Happy with those extra commits too - sometimes we miss things for chapters in flights (like for example if you hadn't been able to generate the HTML for your translation) and the next generate picks them up. So thanks for that 😊

@JustinyAhin
Copy link
Contributor Author

Cool,
I really loved translating the chapter. Thanks to all for your help. It doesn't look like there is a translation to review in the queue. So I'll jump on another chapter.

@tunetheweb
Copy link
Member

Thanks! Go for it.

@JustinyAhin JustinyAhin deleted the JustinyAhin-patch-2 branch January 13, 2020 08:20
@JustinyAhin JustinyAhin restored the JustinyAhin-patch-2 branch January 13, 2020 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
translation world wide web
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants