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

Create nl-application.json #1482

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

johnniezen
Copy link

Translation for Dutch language
I have some errors in the file on lines 111~123, 136, 138, 317 and 318. Not sure and able how to fix.

Translation for Dutch language
Copy link
Contributor

@tomgreenfield tomgreenfield left a comment

Choose a reason for hiding this comment

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

Many thanks for your work on the translation, @johnniezen. Please see my review comments which should fix your JSON errors.

It's worth noting that there are imminent changes to the English version due to be merged into the next release of the tool.

routes/lang/nl-application.json Outdated Show resolved Hide resolved
routes/lang/nl-application.json Outdated Show resolved Hide resolved
routes/lang/nl-application.json Outdated Show resolved Hide resolved
routes/lang/nl-application.json Outdated Show resolved Hide resolved
routes/lang/nl-application.json Outdated Show resolved Hide resolved
"app.submit": "Zenden",
"app.newpassword": "Nieuw wachtwoord",
"app.forgotpasswordblurb": "Voer het e-mail adres in wat bij uw Learning Pool Adapt account hoort, klik dan op Continue. Wij sturen een email met een link waar u makkelijk een nieww wachtwoord maakt.",
"app.forgotpasswordfooter": "Indien u niet langer het e-mail adres wat bij uw Adapt account hoort gebruikt, neem dan contact op met Support voor hulp bij het herstellen van uw account.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove extra space after e-mail adres wat bij uw,

Copy link
Member

Choose a reason for hiding this comment

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

Also question the Learning Pool reference here.

Copy link
Author

Choose a reason for hiding this comment

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

I was not sure if it is still valid, so I left it in. Might it be possible to change it with something like 'your organization' ?

routes/lang/nl-application.json Outdated Show resolved Hide resolved
routes/lang/nl-application.json Outdated Show resolved Hide resolved
routes/lang/nl-application.json Outdated Show resolved Hide resolved
routes/lang/nl-application.json Outdated Show resolved Hide resolved
Copy link
Author

@johnniezen johnniezen left a comment

Choose a reason for hiding this comment

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

Thanks for finding the typos

thx to Tom Greenfield, changed all errors.
tomgreenfield
tomgreenfield previously approved these changes Jan 19, 2017
@taylortom
Copy link
Member

taylortom commented Jan 19, 2017

Many thanks for the PR John!

Couple of things:

  • Which commit did you base the translation on? It doesn't seem to match the latest in the master branch; the references to Learning Pool and Adapt Builder in particular caught my eye.
  • We're very close to releasing a new version of the tool (i.e. days away) which will introduce some new language strings. Would be great if you could add these to your PR.

@johnniezen
Copy link
Author

johnniezen commented Jan 19, 2017 via email

@taylortom
Copy link
Member

taylortom commented Jan 23, 2017

Thanks John!

You'll find the latest version of the file here.
You can see the latest amends in-place by looking at the file's blame page (this may or may not be useful!).

Removed last 4 lines as no longer used in EN version.
@johnniezen
Copy link
Author

Thanks Tom,
I compared the files, and a I could only see the difference in the last 4 lines, which are removed.
Hope it is all good now.

@taylortom
Copy link
Member

@johnniezen I've just set up a quick test, and still getting differences (sorry!). Have set up this gist which will hopefully be of some help.

@johnniezen
Copy link
Author

Hi Tom,
You gist was really helpfull. Hope to have captured all differences now.
Thank you and Tom Greenfield for all the support sofar.
Slowly getting to learn this, but I like it.

@johnniezen
Copy link
Author

Not sure if this is still in Sync. Also looking at the Merging comment, I need to assign 2 approving reviewers. Github help notes something about using a q, but does not specify how the syntax should be.
Anyone to make a similar Gist for my like Tom Taylor did (see a little higher) and tell me how to add reviewers (or volunteer)

@lc-thomasberger lc-thomasberger added the S: awaiting-review Completed issues waiting on reviews label Jan 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S: awaiting-review Completed issues waiting on reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants