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

Translate render-props #73

Merged
merged 16 commits into from Apr 17, 2019
Merged

Translate render-props #73

merged 16 commits into from Apr 17, 2019

Conversation

hg-pyun
Copy link
Member

@hg-pyun hg-pyun commented Mar 16, 2019

@tesseralis tesseralis mentioned this pull request Mar 16, 2019
98 tasks
@hg-pyun hg-pyun added the needs review Needs review A pull request ready to be reviewed label Mar 16, 2019
@hg-pyun hg-pyun assigned hg-pyun and unassigned hg-pyun Mar 16, 2019
@netlify
Copy link

netlify bot commented Mar 16, 2019

Deploy preview for ko-reactjs-org ready!

Built with commit bfbd215

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

@netlify
Copy link

netlify bot commented Mar 16, 2019

Deploy preview for ko-reactjs-org ready!

Built with commit c09fed5

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

@netlify
Copy link

netlify bot commented Mar 16, 2019

Deploy preview for ko-reactjs-org ready!

Built with commit 2d4309d

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

@hg-pyun hg-pyun added needs +1 approval and removed needs review Needs review A pull request ready to be reviewed labels Apr 2, 2019

As a first pass, you might try rendering the `<Cat>` *inside `<Mouse>`'s `render` method*, like this:
첫 번째 방법으로는, 다음과 같이 `<Mouse>` 컴포넌트의 *render 메서드안에* `<Cat>` 컴포넌트를 넣어 렌더링하는 방법이 있습니다:
Copy link
Collaborator

Choose a reason for hiding this comment

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

"있습니다:" -> "있습니다." 콜론 제거되야 할 것 같습니다.

Copy link
Member Author

@hg-pyun hg-pyun Apr 8, 2019

Choose a reason for hiding this comment

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

이부분은 원문을 살려 :를 표현했는데 어느게 좋을까요?
다른 부분에서도 동일하게 컨벤션을 맞추고 있습니다.
image

Copy link
Collaborator

Choose a reason for hiding this comment

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

다른 메인테이너 분들 의견 들어 보는게 좋을 것 같습니다! @taehwanno @taggon @gnujoow

Copy link
Member

Choose a reason for hiding this comment

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

:는 제거하는것이 best practice로 제안되어있습니다. 제거하는것이 컨벤션을 유지하는데 좋을것 같아요

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

@gnujoow 의견 감사합니다 :D

Copy link
Member Author

Choose a reason for hiding this comment

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

반영했습니다 ( _ _ )

@simsim0709 simsim0709 added needs author response Changes requested needing author's input and removed needs +1 approval labels Apr 8, 2019
@hg-pyun hg-pyun added needs +1 approval and removed needs author response Changes requested needing author's input labels Apr 8, 2019
@gnujoow
Copy link
Member

gnujoow commented Apr 11, 2019

#73 (review)
만 resolve되면 바로 merge해도 될것 같습니다.

@hg-pyun
Copy link
Member Author

hg-pyun commented Apr 14, 2019

@gnujoow @simsim0709 @cadenzah 확인 부탁드려요:)

@simsim0709 simsim0709 self-requested a review April 14, 2019 12:28
Copy link
Collaborator

@simsim0709 simsim0709 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다~ 👍

Copy link
Contributor

@cadenzah cadenzah left a comment

Choose a reason for hiding this comment

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

수고하셨습니다! :)

@hg-pyun
Copy link
Member Author

hg-pyun commented Apr 16, 2019

머지 부탁드려요 ㅎㅎ

@hg-pyun hg-pyun added the ready to merge push the merge button :) label Apr 16, 2019
@simsim0709 simsim0709 merged commit d21d361 into reactjs:master Apr 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge push the merge button :)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants