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

Load the data #7

Merged
merged 50 commits into from
Oct 5, 2020
Merged

Load the data #7

merged 50 commits into from
Oct 5, 2020

Conversation

bahobab
Copy link
Owner

@bahobab bahobab commented Oct 2, 2020

No description provided.

…er-bahobab into hero-section

hero-section: Synch remote with local commit
@bahobab bahobab requested a review from ooloo-bot October 4, 2020 09:29
@ooloo-bot
Copy link
Contributor

Great job implementing the load the-data task 🚀 I'm a bit busy at the moment but I'll leave a review soon.

In the meantime I'd suggest you start with the next task already. Even if you just check out a new branch where you can continue working on later. This is a great opportunity to learn professional Git workflows. If you need more information you can find a Git starter tutorial in the knowledge base.

PS: Of course we all know I'm not busy. I'm a bot 🤖 But let's pretend that I am to have a more realistic team environment. We're here to practice after all, right? 😄

Copy link
Contributor

@ooloo-bot ooloo-bot left a comment

Choose a reason for hiding this comment

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

Before I approve this PR I'd like to share a few tips with you.

Please go through the list below and review your own code first. Imagine you're the reviewer or a person who didn't see the code before. Ask yourself if everything is clearly written and understandable. Are components and variables properly named?

Reviewing your own code with this mindset is an important practice that will help you learn a lot and write high-quality code.

To make it a bit easier for you here are the tips I promised. Some have a link 🔗 attached where you can find more information. I'd recommend reading the bold tips in any case. Feel free to check the boxes when you think your code fits.

General:

  • Don't access the DOM directly e.g. via document.getElementById 🔗
  • Make sure variable names are descriptive 🔗
  • Use a recursive function to send 5 requests after one another 🔗
  • Create a custom hook 🔗
  • Don't duplicate state variables 🔗
  • Add error handling 🔗
  • Consider using finally to reset the loading state 🔗
  • Data fetching libraries 🔗
  • Use React Router's useParams to access the URL paramhook to access URL param
  • Use React Router's useHistory hook to update the URL param

Styles:

  • Refrain from styling children with CSS selectors 🔗

Tests:

Once you're done with reviewing your own code feel free to check the example implementation. But remember: It's an important exercise to review your code first on your own.

When you think this PR is ready to be merged just let me know by requesting another review. I'll approve right away.

@bahobab bahobab requested a review from ooloo-bot October 5, 2020 08:05
Copy link
Contributor

@ooloo-bot ooloo-bot 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! 🎉🎉🎉 You can merge the PR now.

@bahobab bahobab merged commit b5167cf into master Oct 5, 2020
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.

2 participants