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

[Fix] Remove duplicated Translations for index.md with unnecessary double quotes #1719

Merged
1 commit merged into from Jul 12, 2018
Merged

[Fix] Remove duplicated Translations for index.md with unnecessary double quotes #1719

1 commit merged into from Jul 12, 2018

Conversation

ghost
Copy link

@ghost ghost commented Jul 8, 2018

  1. Remove duplicated Chinese translations for index.md, there's ONLY
    one explaination.

  2. Remove double quotes for newsletter-postfix. We don't need them and
    the result seems the same.

image

@fhemberger
Copy link
Contributor

/cc @nodejs/nodejs-cn

@ghost
Copy link
Author

ghost commented Jul 8, 2018

@fhemberger
Thanks for marking mine :)

BTW:

  1. Why your @nodejs/nodejs-cn is in BOLD but mine is a common style? I just copied from yours...
  2. Are newsletter-postfix's double quotation marks removed in index.md ? I just had a test and it seems I didn't find anything different before they exist and after they are removed?
    Or do you in fact want to use " instead?

image
image

@fhemberger
Copy link
Contributor

  1. I used the GitHub autocompletion, typing @nodejs…, then it's turned into a notification (it's linked to the group)
  2. Strings in YAML headers don't need quotation marks, they are stripped by the parser.

@ghost ghost requested review from undownding and marswong July 9, 2018 06:44
@ghost ghost self-assigned this Jul 9, 2018
@ghost ghost requested review from undownding and removed request for undownding July 9, 2018 06:48
@ghost
Copy link
Author

ghost commented Jul 9, 2018

PS:Since the main page index.md has been shortened to ONLY ONE explaination of Nodejs. Other translations should also be modified by removing the last pharagraph.

image

/cc:
@nodejs/nodejs-ar
@nodejs/nodejs-tw
@nodejs/nodejs-ja
@nodejs/nodejs-uk
@nodejs/nodejs-ko
@nodejs/nodejs-de
@nodejs/nodejs-it
@nodejs/nodejs-fr
@nodejs/nodejs-es

@@ -18,8 +18,7 @@ labels:
version-schedule-prompt-link-text: LTS 日程。
newsletter: true
newsletter-prefix: 订阅
newsletter-postfix: ",Node.js 官方的新闻周报。"
Copy link
Member

Choose a reason for hiding this comment

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

This change seems to be unrelated.

Copy link
Author

@ghost ghost Jul 10, 2018

Choose a reason for hiding this comment

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

Yes, I just mean that removing the last pharagraph, with the image pasted above for you to see. And there doesn't seem to need quotation marks, so I removed in this fix together.

Copy link
Member

Choose a reason for hiding this comment

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

Then maybe the PR title should be revised to cover that?

Copy link
Author

@ghost ghost Jul 10, 2018

Choose a reason for hiding this comment

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

OK. I'll update it. Thanks!

@vsemozhetbyt
Copy link
Contributor

PR for the locale/uk/index.md: #1726

@ghost ghost changed the title [Fix] Remove duplicated Translations for index.md [Fix] Remove duplicated Translations for index.md with unnecessary double quotes Jul 10, 2018
double quotes

1) Remove duplicated Chinese translations for `index.md`, there's ONLY
one explaination.

2) Remove double quotes for `newsletter-postfix`. We don't need them and
the result seems the same.
This pull request was closed.
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.

3 participants