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.

Sign up for GitHub

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

Jump to bottom

Translate Hooks at a Glance #6

Merged
merged 4 commits into from
Feb 25, 2019
Merged

Translate Hooks at a Glance #6

merged 4 commits into from
Feb 25, 2019

Conversation

chatoo2412
Copy link
Contributor

Please review and give me your feedback. 🙇

@chatoo2412 chatoo2412 mentioned this pull request Feb 8, 2019
Open
gnujoow
gnujoow reviewed Feb 8, 2019
View reviewed changes
Copy link
Member

@gnujoow gnujoow left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I have found some punctuation mistakes.
I havent read original docs and translation yet but it look good to me ;)

personally it is more familiar to read the term singular form in korean.

hooks 개요
hook 개요

content/docs/hooks-overview.md Outdated Show resolved Hide resolved
content/docs/hooks-overview.md Outdated Show resolved Hide resolved
content/docs/hooks-overview.md Outdated Show resolved Hide resolved
content/docs/hooks-overview.md Outdated Show resolved Hide resolved
content/docs/hooks-overview.md Outdated Show resolved Hide resolved
content/docs/hooks-overview.md Outdated Show resolved Hide resolved
content/docs/hooks-overview.md Outdated Show resolved Hide resolved
@chatoo2412
Copy link
Contributor Author

@gnujoow All resolved including the term Hooks. I agree with you that singular form is much familiar to Koreans, but I'm concerned that it's a technical term.

@taehwanno
Copy link
Member

Why this pull request is not executed in CircleCI? I just enabled that before force-push. 😢
In addition, I set to require CircleCI tests to pass for merging into master branch.

@chatoo2412
Copy link
Contributor Author

@taehwanno I'm not sure because I have no permission, please check this: https://circleci.com/docs/2.0/workflows-waiting-status/

@taehwanno
Copy link
Member

Thanks. I read that. but it doesn't work. Temporarily, I disable status checking before merging.

taggon
taggon reviewed Feb 9, 2019
View reviewed changes
content/docs/hooks-overview.md Outdated Show resolved Hide resolved
content/docs/hooks-overview.md Outdated Show resolved Hide resolved
content/docs/hooks-overview.md Outdated Show resolved Hide resolved
content/docs/hooks-overview.md Outdated Show resolved Hide resolved
content/docs/hooks-overview.md Outdated Show resolved Hide resolved
content/docs/hooks-overview.md Outdated Show resolved Hide resolved
content/docs/hooks-overview.md Outdated Show resolved Hide resolved
content/docs/hooks-overview.md Outdated Show resolved Hide resolved
hg-pyun
hg-pyun previously approved these changes Feb 11, 2019
View reviewed changes
Copy link
Member

@hg-pyun hg-pyun left a comment

Choose a reason for hiding this comment

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

LGTM

@tesseralis
Copy link
Member

@hg-pyun are you ready to merge this in?

@hg-pyun
Copy link
Member

hg-pyun commented Feb 14, 2019 •
edited
Loading

@taggon @tesseralis
We'll merge as soon as Taggon makes an approved comment.

@hg-pyun
Copy link
Member

hg-pyun commented Feb 17, 2019

@chatoo2412 Could you resolve conflicts ?

hg-pyun
hg-pyun previously approved these changes Feb 18, 2019
View reviewed changes
taehwanno
taehwanno reviewed Feb 18, 2019
View reviewed changes
Copy link
Member

@taehwanno taehwanno left a comment

Choose a reason for hiding this comment

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

거의 다 된거 같아요 😄 자잘한 수정사항 제안드려봐요!

content/docs/hooks-overview.md Outdated Show resolved Hide resolved
content/docs/hooks-overview.md Outdated Show resolved Hide resolved
content/docs/hooks-overview.md Outdated Show resolved Hide resolved
content/docs/hooks-overview.md Outdated Show resolved Hide resolved
content/docs/hooks-overview.md Outdated Show resolved Hide resolved
content/docs/hooks-overview.md Outdated Show resolved Hide resolved
content/docs/hooks-overview.md Outdated Show resolved Hide resolved
content/docs/hooks-overview.md Outdated Show resolved Hide resolved
content/docs/hooks-overview.md Outdated Show resolved Hide resolved
content/docs/hooks-overview.md Outdated Show resolved Hide resolved
@gnujoow gnujoow dismissed hg-pyun’s stale review February 21, 2019 16:40

수정사항이 반영되지 않아 review를 dismiss합니다. 최종 커밋이 올라온 이후에 review를 다시 진행하면 좋을것 같아요 ;)

@netlify
Copy link

netlify bot commented Feb 23, 2019 •
edited
Loading

Deploy preview for ko-reactjs-org ready!

Built with commit 9c7fe6e

https://deploy-preview-6--ko-reactjs-org.netlify.com

@taehwanno
Copy link
Member

1개 리뷰 누락된 것만 반영되면 이제 거의 다 된 것 같아요 🚀

@taehwanno taehwanno merged commit d34cd21 into reactjs:master Feb 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gnujoow gnujoow gnujoow left review comments

@taggon taggon taggon left review comments

@hg-pyun hg-pyun hg-pyun approved these changes

@taehwanno taehwanno taehwanno approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@chatoo2412 @taehwanno @tesseralis @hg-pyun @taggon @gnujoow