Skip to content

[Translation] Hooks at a Glance #58

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
Merged
merged 37 commits into from
May 26, 2020
Merged

[Translation] Hooks at a Glance #58

merged 37 commits into from
May 26, 2020

Conversation

gautam-pahuja
Copy link
Contributor

No description provided.

@gautam-pahuja gautam-pahuja changed the title [Translation] Hooks Overview [Translation] Hooks at a Glance Nov 21, 2019
@gautam-pahuja gautam-pahuja mentioned this pull request Nov 21, 2019
Closed
88 tasks
@netlify
Copy link

netlify bot commented Nov 21, 2019

Deploy preview for hi-reactjs ready!

Built with commit 078e2d0

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

@netlify
Copy link

netlify bot commented Nov 21, 2019 •
edited
Loading

Deploy preview for hi-reactjs ready!

Built with commit 5cf6c53

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

arshadkazmi42
arshadkazmi42 requested changes Nov 22, 2019
View reviewed changes
Copy link
Member

@arshadkazmi42 arshadkazmi42 left a comment

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

@gautam-pahuja
Copy link
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 requested changes Nov 25, 2019
View reviewed changes
Copy link
Member

@arshadkazmi42 arshadkazmi42 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.

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

@gautam-pahuja
Copy link
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 requested changes Nov 30, 2019
View reviewed changes
Copy link
Member

@arshadkazmi42 arshadkazmi42 left a comment

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

arshadkazmi42
arshadkazmi42 reviewed Dec 1, 2019
View reviewed changes
arshadkazmi42
arshadkazmi42 requested changes Dec 1, 2019
View reviewed changes
Copy link
Member

@arshadkazmi42 arshadkazmi42 left a comment

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
Member

@gautam-pahuja added one more comment

@gautam-pahuja
Copy link
Contributor Author

@gautam-pahuja added one more comment

@arshadkazmi42 Please take a look when you have time.

arshadkazmi42
arshadkazmi42 requested changes Dec 8, 2019
View reviewed changes
Copy link
Member

@arshadkazmi42 arshadkazmi42 left a comment

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

@gautam-pahuja
Copy link
Contributor Author

@arshadkazmi42 Pushed the changes requested.

@arshadkazmi42
Copy link
Member

@gautam-pahuja added some more comments

@gautam-pahuja
Copy link
Contributor Author

@arshadkazmi42 pushed changes.

@arshadkazmi42
Copy link
Member

@gautam-pahuja just one more to go.

@gautam-pahuja
Copy link
Contributor Author

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

@gautam-pahuja
Copy link
Contributor Author

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

@arshadkazmi42
Copy link
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
Member

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

saranshkataria
saranshkataria requested changes Apr 19, 2020
View reviewed changes
Copy link
Member

@saranshkataria saranshkataria left a comment

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}

@gautam-pahuja
Copy link
Contributor Author

@saranshkataria added changes mentioned.

saranshkataria
saranshkataria requested changes May 9, 2020
View reviewed changes
Copy link
Member

@saranshkataria saranshkataria left a comment

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

@gautam-pahuja
Copy link
Contributor Author

@saranshkataria added changes mentioned.

saranshkataria
saranshkataria reviewed May 22, 2020
View reviewed changes
Copy link
Member

@saranshkataria saranshkataria left a comment

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}

@gautam-pahuja
Copy link
Contributor Author

@saranshkataria done.

saranshkataria
saranshkataria reviewed May 25, 2020
View reviewed changes
Copy link
Member

@saranshkataria saranshkataria left a comment

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 🔥

@gautam-pahuja
Copy link
Contributor Author

@saranshkataria Changes added 😄

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

Merged in. Thanks 🎆 🥳

@gautam-pahuja
Copy link
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
Reviewers

@saranshkataria saranshkataria saranshkataria approved these changes

@arshadkazmi42 arshadkazmi42 arshadkazmi42 approved these changes

Assignees
No one assigned
Labels
2nd Review Second phrase of review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gautam-pahuja @arshadkazmi42 @saranshkataria