Skip to content

[Translation] Hooks at a Glance#58

Merged
saranshkataria merged 37 commits into
reactjs:masterfrom
gautam-pahuja:hooks_glance
May 26, 2020
Merged

[Translation] Hooks at a Glance#58
saranshkataria merged 37 commits into
reactjs:masterfrom
gautam-pahuja:hooks_glance

Conversation

@gautam-pahuja

Copy link
Copy Markdown
Contributor

No description provided.

@gautam-pahuja gautam-pahuja changed the title [Translation] Hooks Overview [Translation] Hooks at a Glance Nov 21, 2019
@netlify

netlify Bot commented Nov 21, 2019

Copy link
Copy Markdown

Deploy preview for hi-reactjs ready!

Built with commit 078e2d0

https://deploy-preview-58--hi-reactjs.netlify.com

@netlify

netlify Bot commented Nov 21, 2019

Copy link
Copy Markdown

Deploy preview for hi-reactjs ready!

Built with commit 5cf6c53

https://deploy-preview-58--hi-reactjs.netlify.app

@arshadkazmi42 arshadkazmi42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great start.
Added comments till line 39.
Can you work on the fixes and we will continue with the review post these fixes

Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md
Comment thread content/docs/hooks-overview.md Outdated
@gautam-pahuja

Copy link
Copy Markdown
Contributor Author

Great start.
Added comments till line 39.
Can you work on the fixes and we will continue with the review post these fixes

@arshadkazmi42
I have pushed changes you mentioned and have questions in some of them, please take a look at the unresolved conversations.

@arshadkazmi42 arshadkazmi42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good going.
I have added some more feedbacks till line 101

Also, I have 1 request, do not mark the feedbacks resolved, as reviewer has to mark it as resolve after verifying the changes

Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
@gautam-pahuja

Copy link
Copy Markdown
Contributor Author

Good going.
I have added some more feedbacks till line 101

Also, I have 1 request, do not mark the feedbacks resolved, as reviewer has to mark it as resolve after verifying the changes

@arshadkazmi42
I have made some changes, please take a look.

@arshadkazmi42 arshadkazmi42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Added some more review till line 155

Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated

@arshadkazmi42 arshadkazmi42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great work with fixes.
Added some more suggestions and replied on few comments

@arshadkazmi42

Copy link
Copy Markdown
Member

@gautam-pahuja added one more comment

@gautam-pahuja

Copy link
Copy Markdown
Contributor Author

@gautam-pahuja added one more comment

@arshadkazmi42 Please take a look when you have time.

@arshadkazmi42 arshadkazmi42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great work.
Added review for rest of the files. Its good to go from my end once these are fixed

Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
@gautam-pahuja

Copy link
Copy Markdown
Contributor Author

@arshadkazmi42 Pushed the changes requested.

@arshadkazmi42

Copy link
Copy Markdown
Member

@gautam-pahuja added some more comments

@gautam-pahuja

Copy link
Copy Markdown
Contributor Author

@arshadkazmi42 pushed changes.

@arshadkazmi42

Copy link
Copy Markdown
Member

@gautam-pahuja just one more to go.

@gautam-pahuja

Copy link
Copy Markdown
Contributor Author

@saranshkataria it's been more than a month, do you have anything more to review in this PR?

@gautam-pahuja

Copy link
Copy Markdown
Contributor Author

@arshadkazmi42 any updates needed on this one? It's been lying here for months.

@arshadkazmi42

Copy link
Copy Markdown
Member

@arshadkazmi42 any updates needed on this one? It's been lying here for months.

@saranshkataria will be looking into this from my side its already approved

@saranshkataria

Copy link
Copy Markdown
Contributor

@gautam-pahuja sorry I had been a bit occupied with other things. Reviewing from where I left off and adding comments

@saranshkataria saranshkataria left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Added comments till {#rules-of-hooks}

Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
@gautam-pahuja

Copy link
Copy Markdown
Contributor Author

@saranshkataria added changes mentioned.

Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated

@saranshkataria saranshkataria left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

added comments till line 177

Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
@gautam-pahuja

Copy link
Copy Markdown
Contributor Author

@saranshkataria added changes mentioned.

@saranshkataria saranshkataria left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

added comments till {#other-hooks}

Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
@gautam-pahuja

Copy link
Copy Markdown
Contributor Author

@saranshkataria done.

@saranshkataria saranshkataria left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

added final set of changes. So close 🔥

Comment thread content/docs/hooks-overview.md Outdated
Comment thread content/docs/hooks-overview.md Outdated
@gautam-pahuja

Copy link
Copy Markdown
Contributor Author

@saranshkataria Changes added 😄

@saranshkataria
saranshkataria merged commit 9dcf158 into reactjs:master May 26, 2020
@saranshkataria

Copy link
Copy Markdown
Contributor

Merged in. Thanks 🎆 🥳

@gautam-pahuja

Copy link
Copy Markdown
Contributor Author

Thanks a lot for all the efforts @saranshkataria and @arshadkazmi42 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2nd Review Second phrase of review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants